[llvm] 8851ea6 - [ConstantHoist] Fix APInt ctor assertion

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2024-11-04T11:17:24+01:00
New Revision: 8851ea64a5d01fbf406383f13fcba347e730bc2d

URL: https://github.com/llvm/llvm-project/commit/8851ea64a5d01fbf406383f13fcba347e730bc2d
DIFF: https://github.com/llvm/llvm-project/commit/8851ea64a5d01fbf406383f13fcba347e730bc2d.diff

LOG: [ConstantHoist] Fix APInt ctor assertion

The result here may require truncation. Fix this by removing the
calculateOffsetDiff() helper entirely. As far as I can tell, this
code does not actually have to deal with different bitwidths.

findBaseConstants() will produce ranges of constants with equal
types, which is what maximizeConstantsInRange() will then work
on.

Fixes assertion reported at:
https://github.com/llvm/llvm-project/pull/114539#issuecomment-2453008679

Added: 
    llvm/test/Transforms/ConstantHoisting/ARM/apint-assert.ll

Modified: 
    llvm/lib/Transforms/Scalar/ConstantHoisting.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index dffe5b61a70506..dd37fe2b454138 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -532,25 +532,6 @@ void ConstantHoistingPass::collectConstantCandidates(Function &Fn) {
   }
 }
 
-// This helper function is necessary to deal with values that have 
diff erent
-// bit widths (APInt Operator- does not like that). If the value cannot be
-// represented in uint64 we return an "empty" APInt. This is then interpreted
-// as the value is not in range.
-static std::optional<APInt> calculateOffsetDiff(const APInt &V1,
-                                                const APInt &V2) {
-  std::optional<APInt> Res;
-  unsigned BW = V1.getBitWidth() > V2.getBitWidth() ?
-                V1.getBitWidth() : V2.getBitWidth();
-  uint64_t LimVal1 = V1.getLimitedValue();
-  uint64_t LimVal2 = V2.getLimitedValue();
-
-  if (LimVal1 == ~0ULL || LimVal2 == ~0ULL)
-    return Res;
-
-  uint64_t Diff = LimVal1 - LimVal2;
-  return APInt(BW, Diff, true);
-}
-
 // From a list of constants, one needs to picked as the base and the other
 // constants will be transformed into an offset from that base constant. The
 // question is which we can pick best? For example, consider these constants
@@ -607,16 +588,13 @@ ConstantHoistingPass::maximizeConstantsInRange(ConstCandVecType::iterator S,
       LLVM_DEBUG(dbgs() << "Cost: " << Cost << "\n");
 
       for (auto C2 = S; C2 != E; ++C2) {
-        std::optional<APInt> Diff = calculateOffsetDiff(
-            C2->ConstInt->getValue(), ConstCand->ConstInt->getValue());
-        if (Diff) {
-          const InstructionCost ImmCosts =
-              TTI->getIntImmCodeSizeCost(Opcode, OpndIdx, *Diff, Ty);
-          Cost -= ImmCosts;
-          LLVM_DEBUG(dbgs() << "Offset " << *Diff << " "
-                            << "has penalty: " << ImmCosts << "\n"
-                            << "Adjusted cost: " << Cost << "\n");
-        }
+        APInt Diff = C2->ConstInt->getValue() - ConstCand->ConstInt->getValue();
+        const InstructionCost ImmCosts =
+            TTI->getIntImmCodeSizeCost(Opcode, OpndIdx, Diff, Ty);
+        Cost -= ImmCosts;
+        LLVM_DEBUG(dbgs() << "Offset " << Diff << " "
+                          << "has penalty: " << ImmCosts << "\n"
+                          << "Adjusted cost: " << Cost << "\n");
       }
     }
     LLVM_DEBUG(dbgs() << "Cumulative cost: " << Cost << "\n");

diff  --git a/llvm/test/Transforms/ConstantHoisting/ARM/apint-assert.ll b/llvm/test/Transforms/ConstantHoisting/ARM/apint-assert.ll
new file mode 100644
index 00000000000000..6f165f53a57b2c
--- /dev/null
+++ b/llvm/test/Transforms/ConstantHoisting/ARM/apint-assert.ll
@@ -0,0 +1,18 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=consthoist -mtriple=armv4t-unknown-linux-gnueabi < %s | FileCheck %s
+
+define i1 @test(i32 %arg) optsize {
+; CHECK-LABEL: define i1 @test(
+; CHECK-SAME: i32 [[ARG:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[CONST:%.*]] = bitcast i32 380633088 to i32
+; CHECK-NEXT:    [[CONST_MAT:%.*]] = add i32 [[CONST]], -381681664
+; CHECK-NEXT:    [[SHR_MASK:%.*]] = and i32 [[ARG]], [[CONST_MAT]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[SHR_MASK]], [[CONST]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+entry:
+  %shr.mask = and i32 %arg, -1048576
+  %cmp = icmp eq i32 %shr.mask, 380633088
+  ret i1 %cmp
+}