From 638d285263a5187814aa5e04fe93fe1f8947baaa Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 4 Nov 2024 11:13:50 +0100 Subject: [PATCH] [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 --- .../Transforms/Scalar/ConstantHoisting.cpp | 36 ++++--------------- .../ConstantHoisting/ARM/apint-assert.ll | 18 ++++++++++ 2 files changed, 25 insertions(+), 29 deletions(-) create mode 100644 llvm/test/Transforms/ConstantHoisting/ARM/apint-assert.ll 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 different -// 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 calculateOffsetDiff(const APInt &V1, - const APInt &V2) { - std::optional 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 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 +}