[ADT] Add implementations for mulhs and mulhu to APInt #84609

Sh0g0-1758 · 2024-03-09T06:21:46Z

Fixes: #84207

llvmbot · 2024-03-09T06:22:18Z

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-llvm-support

Author: Shourya Goel (Sh0g0-1758)

Changes

Fixes: #84207

Full diff: https://github.com/llvm/llvm-project/pull/84609.diff

5 Files Affected:

(modified) llvm/include/llvm/ADT/APInt.h (+6)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+4-10)
(modified) llvm/lib/Support/APInt.cpp (+16)
(modified) llvm/unittests/ADT/APIntTest.cpp (+20)
(modified) llvm/unittests/Support/DivisionByConstantTest.cpp (+4-16)

diff --git a/llvm/include/llvm/ADT/APInt.h b/llvm/include/llvm/ADT/APInt.h
index 1fc3c7b2236a17..711eb3a9c3fc6d 100644
--- a/llvm/include/llvm/ADT/APInt.h
+++ b/llvm/include/llvm/ADT/APInt.h
@@ -2193,6 +2193,12 @@ inline const APInt absdiff(const APInt &A, const APInt &B) {
   return A.uge(B) ? (A - B) : (B - A);
 }
 
+/// Compute the higher order bits of unsigned multiplication of two APInts
+APInt mulhu(const APInt &C1, const APInt &C2);
+
+/// Compute the higher order bits of signed multiplication of two APInts
+APInt mulhs(const APInt &C1, const APInt &C2);
+
 /// Compute GCD of two unsigned APInt values.
 ///
 /// This function returns the greatest common divisor of the two APInt values
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 50f53bbb04b62d..4f533b7d055129 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -6015,17 +6015,11 @@ static std::optional<APInt> FoldValue(unsigned Opcode, const APInt &C1,
     if (!C2.getBoolValue())
       break;
     return C1.srem(C2);
-  case ISD::MULHS: {
-    unsigned FullWidth = C1.getBitWidth() * 2;
-    APInt C1Ext = C1.sext(FullWidth);
-    APInt C2Ext = C2.sext(FullWidth);
-    return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
-  }
   case ISD::MULHU: {
-    unsigned FullWidth = C1.getBitWidth() * 2;
-    APInt C1Ext = C1.zext(FullWidth);
-    APInt C2Ext = C2.zext(FullWidth);
-    return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+    return APIntOps::mulhu(C1, C2);
+  }
+  case ISD::MULHS: {
+    return APIntOps::mulhs(C1, C2);
   }
   case ISD::AVGFLOORS: {
     unsigned FullWidth = C1.getBitWidth() + 1;
diff --git a/llvm/lib/Support/APInt.cpp b/llvm/lib/Support/APInt.cpp
index e686b976523302..9ce10ada67e9e1 100644
--- a/llvm/lib/Support/APInt.cpp
+++ b/llvm/lib/Support/APInt.cpp
@@ -3067,6 +3067,22 @@ void llvm::StoreIntToMemory(const APInt &IntVal, uint8_t *Dst,
   }
 }
 
+APInt APIntOps::mulhu(const APInt &C1, const APInt &C2) {
+  // Return higher order bits for unsigned (C1 * C2)
+  unsigned FullWidth = C1.getBitWidth() * 2;
+  APInt C1Ext = C1.zext(FullWidth);
+  APInt C2Ext = C2.zext(FullWidth);
+  return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+}
+
+APInt APIntOps::mulhs(const APInt &C1, const APInt &C2) {
+  // Return higher order bits for signed (C1 * C2)
+  unsigned FullWidth = C1.getBitWidth() * 2;
+  APInt C1Ext = C1.sext(FullWidth);
+  APInt C2Ext = C2.sext(FullWidth);
+  return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+}
+
 /// LoadIntFromMemory - Loads the integer stored in the LoadBytes bytes starting
 /// from Src into IntVal, which is assumed to be wide enough and to hold zero.
 void llvm::LoadIntFromMemory(APInt &IntVal, const uint8_t *Src,
diff --git a/llvm/unittests/ADT/APIntTest.cpp b/llvm/unittests/ADT/APIntTest.cpp
index 24324822356bf6..9995aeaff92871 100644
--- a/llvm/unittests/ADT/APIntTest.cpp
+++ b/llvm/unittests/ADT/APIntTest.cpp
@@ -2805,6 +2805,26 @@ TEST(APIntTest, multiply) {
   EXPECT_EQ(64U, i96.countr_zero());
 }
 
+TEST(APIntTest, Hmultiply) {
+  APInt i1048576(32, 1048576);
+
+  EXPECT_EQ(APInt(32, 256), APIntOps::mulhu(i1048576, i1048576));
+
+  APInt i16777216(32, 16777216);
+  APInt i32768(32, 32768);
+
+  EXPECT_EQ(APInt(32, 128), APIntOps::mulhu(i16777216, i32768));
+  EXPECT_EQ(APInt(32, 128), APIntOps::mulhu(i32768, i16777216));
+
+  APInt i2097152(32, -2097152);
+  APInt i524288(32, 524288);
+
+  EXPECT_EQ(APInt(32, 1024), APIntOps::mulhs(i2097152, i2097152));
+
+  EXPECT_EQ(APInt(32, -256), APIntOps::mulhs(i2097152, i524288));
+  EXPECT_EQ(APInt(32, -256), APIntOps::mulhs(i524288, i2097152));
+}
+
 TEST(APIntTest, RoundingUDiv) {
   for (uint64_t Ai = 1; Ai <= 255; Ai++) {
     APInt A(8, Ai);
diff --git a/llvm/unittests/Support/DivisionByConstantTest.cpp b/llvm/unittests/Support/DivisionByConstantTest.cpp
index 2b17f98bb75b2f..8e0c78fe85654a 100644
--- a/llvm/unittests/Support/DivisionByConstantTest.cpp
+++ b/llvm/unittests/Support/DivisionByConstantTest.cpp
@@ -21,12 +21,6 @@ template <typename Fn> static void EnumerateAPInts(unsigned Bits, Fn TestFn) {
   } while (++N != 0);
 }
 
-APInt MULHS(APInt X, APInt Y) {
-  unsigned Bits = X.getBitWidth();
-  unsigned WideBits = 2 * Bits;
-  return (X.sext(WideBits) * Y.sext(WideBits)).lshr(Bits).trunc(Bits);
-}
-
 APInt SignedDivideUsingMagic(APInt Numerator, APInt Divisor,
                              SignedDivisionByConstantInfo Magics) {
   unsigned Bits = Numerator.getBitWidth();
@@ -48,7 +42,7 @@ APInt SignedDivideUsingMagic(APInt Numerator, APInt Divisor,
   }
 
   // Multiply the numerator by the magic value.
-  APInt Q = MULHS(Numerator, Magics.Magic);
+  APInt Q = APIntOps::mulhs(Numerator, Magics.Magic);
 
   // (Optionally) Add/subtract the numerator using Factor.
   Factor = Numerator * Factor;
@@ -89,12 +83,6 @@ TEST(SignedDivisionByConstantTest, Test) {
   }
 }
 
-APInt MULHU(APInt X, APInt Y) {
-  unsigned Bits = X.getBitWidth();
-  unsigned WideBits = 2 * Bits;
-  return (X.zext(WideBits) * Y.zext(WideBits)).lshr(Bits).trunc(Bits);
-}
-
 APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
                                bool LZOptimization,
                                bool AllowEvenDivisorOptimization, bool ForceNPQ,
@@ -129,16 +117,16 @@ APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
   APInt Q = Numerator.lshr(PreShift);
 
   // Multiply the numerator by the magic value.
-  Q = MULHU(Q, Magics.Magic);
+  Q = APIntOps::mulhu(Q, Magics.Magic);
 
   if (UseNPQ || ForceNPQ) {
     APInt NPQ = Numerator - Q;
 
     // For vectors we might have a mix of non-NPQ/NPQ paths, so use
-    // MULHU to act as a SRL-by-1 for NPQ, else multiply by zero.
+    // mulhu to act as a SRL-by-1 for NPQ, else multiply by zero.
     APInt NPQ_Scalar = NPQ.lshr(1);
     (void)NPQ_Scalar;
-    NPQ = MULHU(NPQ, NPQFactor);
+    NPQ = APIntOps::mulhu(NPQ, NPQFactor);
     assert(!UseNPQ || NPQ == NPQ_Scalar);
 
     Q = NPQ + Q;

llvmbot · 2024-03-09T06:22:18Z

@llvm/pr-subscribers-llvm-adt

Author: Shourya Goel (Sh0g0-1758)

Changes

Fixes: #84207

Full diff: https://github.com/llvm/llvm-project/pull/84609.diff

5 Files Affected:

(modified) llvm/include/llvm/ADT/APInt.h (+6)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+4-10)
(modified) llvm/lib/Support/APInt.cpp (+16)
(modified) llvm/unittests/ADT/APIntTest.cpp (+20)
(modified) llvm/unittests/Support/DivisionByConstantTest.cpp (+4-16)

diff --git a/llvm/include/llvm/ADT/APInt.h b/llvm/include/llvm/ADT/APInt.h
index 1fc3c7b2236a17..711eb3a9c3fc6d 100644
--- a/llvm/include/llvm/ADT/APInt.h
+++ b/llvm/include/llvm/ADT/APInt.h
@@ -2193,6 +2193,12 @@ inline const APInt absdiff(const APInt &A, const APInt &B) {
   return A.uge(B) ? (A - B) : (B - A);
 }
 
+/// Compute the higher order bits of unsigned multiplication of two APInts
+APInt mulhu(const APInt &C1, const APInt &C2);
+
+/// Compute the higher order bits of signed multiplication of two APInts
+APInt mulhs(const APInt &C1, const APInt &C2);
+
 /// Compute GCD of two unsigned APInt values.
 ///
 /// This function returns the greatest common divisor of the two APInt values
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 50f53bbb04b62d..4f533b7d055129 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -6015,17 +6015,11 @@ static std::optional<APInt> FoldValue(unsigned Opcode, const APInt &C1,
     if (!C2.getBoolValue())
       break;
     return C1.srem(C2);
-  case ISD::MULHS: {
-    unsigned FullWidth = C1.getBitWidth() * 2;
-    APInt C1Ext = C1.sext(FullWidth);
-    APInt C2Ext = C2.sext(FullWidth);
-    return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
-  }
   case ISD::MULHU: {
-    unsigned FullWidth = C1.getBitWidth() * 2;
-    APInt C1Ext = C1.zext(FullWidth);
-    APInt C2Ext = C2.zext(FullWidth);
-    return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+    return APIntOps::mulhu(C1, C2);
+  }
+  case ISD::MULHS: {
+    return APIntOps::mulhs(C1, C2);
   }
   case ISD::AVGFLOORS: {
     unsigned FullWidth = C1.getBitWidth() + 1;
diff --git a/llvm/lib/Support/APInt.cpp b/llvm/lib/Support/APInt.cpp
index e686b976523302..9ce10ada67e9e1 100644
--- a/llvm/lib/Support/APInt.cpp
+++ b/llvm/lib/Support/APInt.cpp
@@ -3067,6 +3067,22 @@ void llvm::StoreIntToMemory(const APInt &IntVal, uint8_t *Dst,
   }
 }
 
+APInt APIntOps::mulhu(const APInt &C1, const APInt &C2) {
+  // Return higher order bits for unsigned (C1 * C2)
+  unsigned FullWidth = C1.getBitWidth() * 2;
+  APInt C1Ext = C1.zext(FullWidth);
+  APInt C2Ext = C2.zext(FullWidth);
+  return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+}
+
+APInt APIntOps::mulhs(const APInt &C1, const APInt &C2) {
+  // Return higher order bits for signed (C1 * C2)
+  unsigned FullWidth = C1.getBitWidth() * 2;
+  APInt C1Ext = C1.sext(FullWidth);
+  APInt C2Ext = C2.sext(FullWidth);
+  return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+}
+
 /// LoadIntFromMemory - Loads the integer stored in the LoadBytes bytes starting
 /// from Src into IntVal, which is assumed to be wide enough and to hold zero.
 void llvm::LoadIntFromMemory(APInt &IntVal, const uint8_t *Src,
diff --git a/llvm/unittests/ADT/APIntTest.cpp b/llvm/unittests/ADT/APIntTest.cpp
index 24324822356bf6..9995aeaff92871 100644
--- a/llvm/unittests/ADT/APIntTest.cpp
+++ b/llvm/unittests/ADT/APIntTest.cpp
@@ -2805,6 +2805,26 @@ TEST(APIntTest, multiply) {
   EXPECT_EQ(64U, i96.countr_zero());
 }
 
+TEST(APIntTest, Hmultiply) {
+  APInt i1048576(32, 1048576);
+
+  EXPECT_EQ(APInt(32, 256), APIntOps::mulhu(i1048576, i1048576));
+
+  APInt i16777216(32, 16777216);
+  APInt i32768(32, 32768);
+
+  EXPECT_EQ(APInt(32, 128), APIntOps::mulhu(i16777216, i32768));
+  EXPECT_EQ(APInt(32, 128), APIntOps::mulhu(i32768, i16777216));
+
+  APInt i2097152(32, -2097152);
+  APInt i524288(32, 524288);
+
+  EXPECT_EQ(APInt(32, 1024), APIntOps::mulhs(i2097152, i2097152));
+
+  EXPECT_EQ(APInt(32, -256), APIntOps::mulhs(i2097152, i524288));
+  EXPECT_EQ(APInt(32, -256), APIntOps::mulhs(i524288, i2097152));
+}
+
 TEST(APIntTest, RoundingUDiv) {
   for (uint64_t Ai = 1; Ai <= 255; Ai++) {
     APInt A(8, Ai);
diff --git a/llvm/unittests/Support/DivisionByConstantTest.cpp b/llvm/unittests/Support/DivisionByConstantTest.cpp
index 2b17f98bb75b2f..8e0c78fe85654a 100644
--- a/llvm/unittests/Support/DivisionByConstantTest.cpp
+++ b/llvm/unittests/Support/DivisionByConstantTest.cpp
@@ -21,12 +21,6 @@ template <typename Fn> static void EnumerateAPInts(unsigned Bits, Fn TestFn) {
   } while (++N != 0);
 }
 
-APInt MULHS(APInt X, APInt Y) {
-  unsigned Bits = X.getBitWidth();
-  unsigned WideBits = 2 * Bits;
-  return (X.sext(WideBits) * Y.sext(WideBits)).lshr(Bits).trunc(Bits);
-}
-
 APInt SignedDivideUsingMagic(APInt Numerator, APInt Divisor,
                              SignedDivisionByConstantInfo Magics) {
   unsigned Bits = Numerator.getBitWidth();
@@ -48,7 +42,7 @@ APInt SignedDivideUsingMagic(APInt Numerator, APInt Divisor,
   }
 
   // Multiply the numerator by the magic value.
-  APInt Q = MULHS(Numerator, Magics.Magic);
+  APInt Q = APIntOps::mulhs(Numerator, Magics.Magic);
 
   // (Optionally) Add/subtract the numerator using Factor.
   Factor = Numerator * Factor;
@@ -89,12 +83,6 @@ TEST(SignedDivisionByConstantTest, Test) {
   }
 }
 
-APInt MULHU(APInt X, APInt Y) {
-  unsigned Bits = X.getBitWidth();
-  unsigned WideBits = 2 * Bits;
-  return (X.zext(WideBits) * Y.zext(WideBits)).lshr(Bits).trunc(Bits);
-}
-
 APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
                                bool LZOptimization,
                                bool AllowEvenDivisorOptimization, bool ForceNPQ,
@@ -129,16 +117,16 @@ APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
   APInt Q = Numerator.lshr(PreShift);
 
   // Multiply the numerator by the magic value.
-  Q = MULHU(Q, Magics.Magic);
+  Q = APIntOps::mulhu(Q, Magics.Magic);
 
   if (UseNPQ || ForceNPQ) {
     APInt NPQ = Numerator - Q;
 
     // For vectors we might have a mix of non-NPQ/NPQ paths, so use
-    // MULHU to act as a SRL-by-1 for NPQ, else multiply by zero.
+    // mulhu to act as a SRL-by-1 for NPQ, else multiply by zero.
     APInt NPQ_Scalar = NPQ.lshr(1);
     (void)NPQ_Scalar;
-    NPQ = MULHU(NPQ, NPQFactor);
+    NPQ = APIntOps::mulhu(NPQ, NPQFactor);
     assert(!UseNPQ || NPQ == NPQ_Scalar);
 
     Q = NPQ + Q;

jayfoad

LGTM.

The issue also suggests updating KnownBitsTest.cpp.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Sh0g0-1758 · 2024-03-09T10:42:19Z

@jayfoad, regarding changes in KnownBitsTest.cpp, is it expected to change return KnownBits::mulhs(Known1, Known2); with return APIntOps::mulhs(Known1, Known2)? Since KnownBits have its own implementation of mulhs and mulhu, I am bit confused if that is the right thing to do.

jayfoad · 2024-03-09T12:24:24Z

@jayfoad, regarding changes in KnownBitsTest.cpp, is it expected to change return KnownBits::mulhs(Known1, Known2); with return APIntOps::mulhs(Known1, Known2)?

No, it's this expression in the mulhs test (and similar for mulhu) that can be replaced with a call to the new APInt method: (N1.sext(2 * Bits) * N2.sext(2 * Bits)).extractBits(Bits, Bits)

Sh0g0-1758 · 2024-03-09T12:34:22Z

Made the changes. I was under the impression that something similar to DivisionByConstantTest.cpp needed to be changed, so that did not catch my attention.

Sh0g0-1758 · 2024-03-09T13:43:41Z

@jayfoad, some Flang tests are failing for the Windows build. I know this is a pretty common issue, though not related to the PR. If there are no further suggestions, I guess it's safe to land?

RKSimon

A couple of minors

RKSimon · 2024-03-09T16:15:25Z

llvm/lib/Support/APInt.cpp

@@ -3067,6 +3067,22 @@ void llvm::StoreIntToMemory(const APInt &IntVal, uint8_t *Dst,
  }
 }

+APInt APIntOps::mulhu(const APInt &C1, const APInt &C2) {


add same-bitwidth assertion: assert(C1.BitWidth == C2.BitWidth && "Bit widths must be the same");

It should be C1.getBitWidth(). Updated the code with it.

RKSimon · 2024-03-09T16:15:32Z

llvm/lib/Support/APInt.cpp

+  return (C1Ext * C2Ext).extractBits(C1.getBitWidth(), C1.getBitWidth());
+}
+
+APInt APIntOps::mulhs(const APInt &C1, const APInt &C2) {


add same-bitwidth assertion: assert(C1.BitWidth == C2.BitWidth && "Bit widths must be the same");

Sh0g0-1758 · 2024-03-09T18:38:21Z

@RKSimon, reverting the assertion statements because DivisionByConstantTest won't pass if it is left implemented, since it has

template <typename Fn> static void EnumerateAPInts(unsigned Bits, Fn TestFn) {
  APInt N(Bits, 0);
  do {
    TestFn(N);
  } while (++N != 0);
}

Which means the number of bits of the two APInts can't be same. Please correct me if I am wrong.

kuhar

Nice! We have a few more places in mli that could be switched to this:

llvm-project/mlir/lib/Dialect/SPIRV/IR/SPIRVCanonicalization.cpp

Lines 251 to 264 in 110141b

    
           auto highBits = constFoldBinaryOp<IntegerAttr>( 
        
               {lhsAttr, rhsAttr}, [](const APInt &a, const APInt &b) { 
        
                 unsigned bitWidth = a.getBitWidth(); 
        
                 APInt c; 
        
                 if (IsSigned) { 
        
                   c = a.sext(bitWidth * 2) * b.sext(bitWidth * 2); 
        
                 } else { 
        
                   c = a.zext(bitWidth * 2) * b.zext(bitWidth * 2); 
        
                 } 
        
                 return c.extractBits(bitWidth, bitWidth); // Extract high result 
        
               }); 
        
           if (!highBits) 
        
             return failure();

llvm-project/mlir/lib/Dialect/Arith/IR/ArithOps.cpp

Lines 430 to 445 in 110141b

    
           // mulsi_extended(cst_a, cst_b) -> cst_low, cst_high 
        
           if (Attribute lowAttr = constFoldBinaryOp<IntegerAttr>( 
        
                   adaptor.getOperands(), 
        
                   [](const APInt &a, const APInt &b) { return a * b; })) { 
        
             // Invoke the constant fold helper again to calculate the 'high' result. 
        
             Attribute highAttr = constFoldBinaryOp<IntegerAttr>( 
        
                 adaptor.getOperands(), [](const APInt &a, const APInt &b) { 
        
                   unsigned bitWidth = a.getBitWidth(); 
        
                   APInt fullProduct = a.sext(bitWidth * 2) * b.sext(bitWidth * 2); 
        
                   return fullProduct.extractBits(bitWidth, bitWidth); 
        
                 }); 
        
             assert(highAttr && "Unexpected constant-folding failure"); 
        
             results.push_back(lowAttr); 
        
             results.push_back(highAttr); 
        
             return success();

llvm-project/mlir/lib/Dialect/Arith/IR/ArithOps.cpp

Lines 487 to 502 in 110141b

    
           // mului_extended(cst_a, cst_b) -> cst_low, cst_high 
        
           if (Attribute lowAttr = constFoldBinaryOp<IntegerAttr>( 
        
                   adaptor.getOperands(), 
        
                   [](const APInt &a, const APInt &b) { return a * b; })) { 
        
             // Invoke the constant fold helper again to calculate the 'high' result. 
        
             Attribute highAttr = constFoldBinaryOp<IntegerAttr>( 
        
                 adaptor.getOperands(), [](const APInt &a, const APInt &b) { 
        
                   unsigned bitWidth = a.getBitWidth(); 
        
                   APInt fullProduct = a.zext(bitWidth * 2) * b.zext(bitWidth * 2); 
        
                   return fullProduct.extractBits(bitWidth, bitWidth); 
        
                 }); 
        
             assert(highAttr && "Unexpected constant-folding failure"); 
        
             results.push_back(lowAttr); 
        
             results.push_back(highAttr); 
        
             return success();

llvm/include/llvm/ADT/APInt.h

llvm/unittests/Support/KnownBitsTest.cpp

github-actions · 2024-03-09T20:54:01Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/include/llvm/ADT/APInt.h

mlir/lib/Dialect/Arith/IR/ArithOps.cpp

Co-authored-by: Jakub Kuderski <[email protected]>

Sh0g0-1758 · 2024-03-09T22:12:50Z

@kuhar, I have implemented the MLIR changes and the tests are passing. For the signed comments, I tried to keep it concise, I wrote a kinda draft for the comments but it was taking up a lot of space, so decided to keep it short.

kuhar

LGTM, thanks for the cleanups

Sh0g0-1758 · 2024-03-10T06:39:20Z

@RKSimon, could you drop a confirmation about assertion statements? I think apart from that, this looks good to land.

RKSimon · 2024-03-10T15:37:32Z

@Sh0g0-1758 I thought the conclusion of the discussion of #84207 was that @Atousa was working on this?

Sh0g0-1758 · 2024-03-10T15:56:29Z

Hm.. I was under the impression that since the work here is done, she won't be working on this but from now on we keep a 7 day period before pinging on good first issues. I also don't see a PR from her yet. But to be honest, this is becoming way too complex, I really think if the work here is done, we should land it.

Sh0g0-1758 · 2024-03-10T16:43:05Z

and if it's not done yet, I suppose she can collaborate on this with me with the remaining work. I do think that the main goal when doing open-source contributions should be learning and collaboration and not focusing on just solving an issue.

Sh0g0-1758 · 2024-03-10T18:45:42Z

Closing this bcz of conversation in original issue thread.

kuhar · 2024-03-10T19:24:52Z

@Sh0g0-1758 I don't have the wider context but this PR looks like a nice code cleanup on its own.

Sh0g0-1758 · 2024-03-11T03:48:02Z

I see, but I suppose another dev is going to raise a similar PR soon.

Sh0g0-1758 added 2 commits March 9, 2024 11:40

Add APIntOps::mulhs / APIntOps::mulhu

5478873

Removed older case statements

abdc2f8

llvmbot added llvm:support llvm:SelectionDAG SelectionDAGISel as well llvm:adt labels Mar 9, 2024

jayfoad approved these changes Mar 9, 2024

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Outdated Show resolved Hide resolved

Sh0g0-1758 added 2 commits March 9, 2024 16:08

Removed Braces

c81a474

Ran clang formatter

bd7cd46

Sh0g0-1758 added 2 commits March 9, 2024 18:01

Refactored KnownBitsTest

32ec343

Ran clang formatter

3b7b3a4

jayfoad approved these changes Mar 9, 2024

View reviewed changes

jayfoad requested a review from RKSimon March 9, 2024 15:55

RKSimon reviewed Mar 9, 2024

View reviewed changes

Sh0g0-1758 added 2 commits March 9, 2024 21:52

added same-bitwidth assertion

2d6804a

Minor Bug Fix

b80b94d

revert

2bd2d18

kuhar reviewed Mar 9, 2024

View reviewed changes

llvm/include/llvm/ADT/APInt.h Outdated Show resolved Hide resolved

llvm/include/llvm/ADT/APInt.h Outdated Show resolved Hide resolved

llvm/unittests/Support/KnownBitsTest.cpp Show resolved Hide resolved

Sh0g0-1758 added 2 commits March 10, 2024 02:21

Added Docs and Refactored Functions

0d3edd5

Ran clang Formatter

6fb4b4a

Sh0g0-1758 added 2 commits March 10, 2024 02:32

Updated MLIR files

02d896e

Ran clang Formatter

abce2ad

Sh0g0-1758 requested a review from antiagainst as a code owner March 9, 2024 21:17

Sh0g0-1758 added 2 commits March 10, 2024 02:54

Add namespace

6816c6a

Ran Clang Formatter

4f0a94a

kuhar reviewed Mar 9, 2024

View reviewed changes

Sh0g0-1758 and others added 6 commits March 10, 2024 03:00

Update llvm/include/llvm/ADT/APInt.h

6e24505

Co-authored-by: Jakub Kuderski <[email protected]>

Update llvm/include/llvm/ADT/APInt.h

f94a279

Co-authored-by: Jakub Kuderski <[email protected]>

Replaced Lamda Function

db910c4

Ran clang Formatter

58f0613

Bringing back lamda functions

8aee6cd

Ran Clang Formatter

a60ffdc

kuhar approved these changes Mar 9, 2024

View reviewed changes

Sh0g0-1758 closed this Mar 10, 2024

kuhar mentioned this pull request Mar 12, 2024

[ADT] Add signed and unsigned mulh to APInt #84719

Merged

Sh0g0-1758 deleted the 84207 branch March 13, 2024 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADT] Add implementations for mulhs and mulhu to APInt #84609

[ADT] Add implementations for mulhs and mulhu to APInt #84609

Sh0g0-1758 commented Mar 9, 2024

llvmbot commented Mar 9, 2024 •

edited

Loading

llvmbot commented Mar 9, 2024

jayfoad left a comment

Sh0g0-1758 commented Mar 9, 2024

jayfoad commented Mar 9, 2024

Sh0g0-1758 commented Mar 9, 2024 •

edited

Loading

Sh0g0-1758 commented Mar 9, 2024

RKSimon left a comment

RKSimon Mar 9, 2024

Sh0g0-1758 Mar 9, 2024

RKSimon Mar 9, 2024

Sh0g0-1758 Mar 9, 2024

Sh0g0-1758 commented Mar 9, 2024

kuhar left a comment

github-actions bot commented Mar 9, 2024 •

edited

Loading

Sh0g0-1758 commented Mar 9, 2024

kuhar left a comment

Sh0g0-1758 commented Mar 10, 2024

RKSimon commented Mar 10, 2024

Sh0g0-1758 commented Mar 10, 2024 •

edited

Loading

Sh0g0-1758 commented Mar 10, 2024

Sh0g0-1758 commented Mar 10, 2024

kuhar commented Mar 10, 2024

Sh0g0-1758 commented Mar 11, 2024

	auto highBits = constFoldBinaryOp<IntegerAttr>(
	{lhsAttr, rhsAttr}, [](const APInt &a, const APInt &b) {
	unsigned bitWidth = a.getBitWidth();
	APInt c;
	if (IsSigned) {
	c = a.sext(bitWidth * 2) * b.sext(bitWidth * 2);
	} else {
	c = a.zext(bitWidth * 2) * b.zext(bitWidth * 2);
	}
	return c.extractBits(bitWidth, bitWidth); // Extract high result
	});

	if (!highBits)
	return failure();

	// mulsi_extended(cst_a, cst_b) -> cst_low, cst_high
	if (Attribute lowAttr = constFoldBinaryOp<IntegerAttr>(
	adaptor.getOperands(),
	[](const APInt &a, const APInt &b) { return a * b; })) {
	// Invoke the constant fold helper again to calculate the 'high' result.
	Attribute highAttr = constFoldBinaryOp<IntegerAttr>(
	adaptor.getOperands(), [](const APInt &a, const APInt &b) {
	unsigned bitWidth = a.getBitWidth();
	APInt fullProduct = a.sext(bitWidth * 2) * b.sext(bitWidth * 2);
	return fullProduct.extractBits(bitWidth, bitWidth);
	});
	assert(highAttr && "Unexpected constant-folding failure");

	results.push_back(lowAttr);
	results.push_back(highAttr);
	return success();

	// mului_extended(cst_a, cst_b) -> cst_low, cst_high
	if (Attribute lowAttr = constFoldBinaryOp<IntegerAttr>(
	adaptor.getOperands(),
	[](const APInt &a, const APInt &b) { return a * b; })) {
	// Invoke the constant fold helper again to calculate the 'high' result.
	Attribute highAttr = constFoldBinaryOp<IntegerAttr>(
	adaptor.getOperands(), [](const APInt &a, const APInt &b) {
	unsigned bitWidth = a.getBitWidth();
	APInt fullProduct = a.zext(bitWidth * 2) * b.zext(bitWidth * 2);
	return fullProduct.extractBits(bitWidth, bitWidth);
	});
	assert(highAttr && "Unexpected constant-folding failure");

	results.push_back(lowAttr);
	results.push_back(highAttr);
	return success();

[ADT] Add implementations for mulhs and mulhu to APInt #84609

[ADT] Add implementations for mulhs and mulhu to APInt #84609

Conversation

Sh0g0-1758 commented Mar 9, 2024

llvmbot commented Mar 9, 2024 • edited Loading

llvmbot commented Mar 9, 2024

jayfoad left a comment

Choose a reason for hiding this comment

Sh0g0-1758 commented Mar 9, 2024

jayfoad commented Mar 9, 2024

Sh0g0-1758 commented Mar 9, 2024 • edited Loading

Sh0g0-1758 commented Mar 9, 2024

RKSimon left a comment

Choose a reason for hiding this comment

RKSimon Mar 9, 2024

Choose a reason for hiding this comment

Sh0g0-1758 Mar 9, 2024

Choose a reason for hiding this comment

RKSimon Mar 9, 2024

Choose a reason for hiding this comment

Sh0g0-1758 Mar 9, 2024

Choose a reason for hiding this comment

Sh0g0-1758 commented Mar 9, 2024

kuhar left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 9, 2024 • edited Loading

Sh0g0-1758 commented Mar 9, 2024

kuhar left a comment

Choose a reason for hiding this comment

Sh0g0-1758 commented Mar 10, 2024

RKSimon commented Mar 10, 2024

Sh0g0-1758 commented Mar 10, 2024 • edited Loading

Sh0g0-1758 commented Mar 10, 2024

Sh0g0-1758 commented Mar 10, 2024

kuhar commented Mar 10, 2024

Sh0g0-1758 commented Mar 11, 2024

llvmbot commented Mar 9, 2024 •

edited

Loading

Sh0g0-1758 commented Mar 9, 2024 •

edited

Loading

github-actions bot commented Mar 9, 2024 •

edited

Loading

Sh0g0-1758 commented Mar 10, 2024 •

edited

Loading