Skip to content

Commit a953ef8

Browse files
nikicakiramenai
authored andcommitted
[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: llvm/llvm-project#114539 (comment)
1 parent 83f72af commit a953ef8

File tree

2 files changed

+25
-29
lines changed

2 files changed

+25
-29
lines changed

llvm/lib/Transforms/Scalar/ConstantHoisting.cpp

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -533,25 +533,6 @@ void ConstantHoistingPass::collectConstantCandidates(Function &Fn) {
533533
}
534534
}
535535

536-
// This helper function is necessary to deal with values that have different
537-
// bit widths (APInt Operator- does not like that). If the value cannot be
538-
// represented in uint64 we return an "empty" APInt. This is then interpreted
539-
// as the value is not in range.
540-
static std::optional<APInt> calculateOffsetDiff(const APInt &V1,
541-
const APInt &V2) {
542-
std::optional<APInt> Res;
543-
unsigned BW = V1.getBitWidth() > V2.getBitWidth() ?
544-
V1.getBitWidth() : V2.getBitWidth();
545-
uint64_t LimVal1 = V1.getLimitedValue();
546-
uint64_t LimVal2 = V2.getLimitedValue();
547-
548-
if (LimVal1 == ~0ULL || LimVal2 == ~0ULL)
549-
return Res;
550-
551-
uint64_t Diff = LimVal1 - LimVal2;
552-
return APInt(BW, Diff, true);
553-
}
554-
555536
// From a list of constants, one needs to picked as the base and the other
556537
// constants will be transformed into an offset from that base constant. The
557538
// question is which we can pick best? For example, consider these constants
@@ -608,16 +589,13 @@ ConstantHoistingPass::maximizeConstantsInRange(ConstCandVecType::iterator S,
608589
LLVM_DEBUG(dbgs() << "Cost: " << Cost << "\n");
609590

610591
for (auto C2 = S; C2 != E; ++C2) {
611-
std::optional<APInt> Diff = calculateOffsetDiff(
612-
C2->ConstInt->getValue(), ConstCand->ConstInt->getValue());
613-
if (Diff) {
614-
const InstructionCost ImmCosts =
615-
TTI->getIntImmCodeSizeCost(Opcode, OpndIdx, *Diff, Ty);
616-
Cost -= ImmCosts;
617-
LLVM_DEBUG(dbgs() << "Offset " << *Diff << " "
618-
<< "has penalty: " << ImmCosts << "\n"
619-
<< "Adjusted cost: " << Cost << "\n");
620-
}
592+
APInt Diff = C2->ConstInt->getValue() - ConstCand->ConstInt->getValue();
593+
const InstructionCost ImmCosts =
594+
TTI->getIntImmCodeSizeCost(Opcode, OpndIdx, Diff, Ty);
595+
Cost -= ImmCosts;
596+
LLVM_DEBUG(dbgs() << "Offset " << Diff << " "
597+
<< "has penalty: " << ImmCosts << "\n"
598+
<< "Adjusted cost: " << Cost << "\n");
621599
}
622600
}
623601
LLVM_DEBUG(dbgs() << "Cumulative cost: " << Cost << "\n");
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -S -passes=consthoist -mtriple=armv4t-unknown-linux-gnueabi < %s | FileCheck %s
3+
4+
define i1 @test(i32 %arg) optsize {
5+
; CHECK-LABEL: define i1 @test(
6+
; CHECK-SAME: i32 [[ARG:%.*]]) #[[ATTR0:[0-9]+]] {
7+
; CHECK-NEXT: [[ENTRY:.*:]]
8+
; CHECK-NEXT: [[CONST:%.*]] = bitcast i32 380633088 to i32
9+
; CHECK-NEXT: [[CONST_MAT:%.*]] = add i32 [[CONST]], -381681664
10+
; CHECK-NEXT: [[SHR_MASK:%.*]] = and i32 [[ARG]], [[CONST_MAT]]
11+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[SHR_MASK]], [[CONST]]
12+
; CHECK-NEXT: ret i1 [[CMP]]
13+
;
14+
entry:
15+
%shr.mask = and i32 %arg, -1048576
16+
%cmp = icmp eq i32 %shr.mask, 380633088
17+
ret i1 %cmp
18+
}

0 commit comments

Comments
 (0)