Skip to content

Commit

Permalink
[ConstantHoist] Fix APInt ctor assertion
Browse files Browse the repository at this point in the history
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:
llvm#114539 (comment)
  • Loading branch information
nikic committed Nov 4, 2024
1 parent daa9af1 commit 8851ea6
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
36 changes: 7 additions & 29 deletions llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<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
Expand Down Expand Up @@ -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");
Expand Down
18 changes: 18 additions & 0 deletions llvm/test/Transforms/ConstantHoisting/ARM/apint-assert.ll
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 8851ea6

Please sign in to comment.