Skip to content

Commit 1d5d125

Browse files
authored
[ConstantFolding] Consolidate poison propagation for intrinsics (#146878)
This consolidates the "fold poison arg to poison result" constant folding logic for intrinsics, based on a common intrinsicPropagatesPoison() helper, which is also used for poison propagation reasoning in ValueTracking. This ensures that the set of supported intrinsics is consistent. This add ucmp, scmp, smul.fix, smul.fix.sat, canonicalize and sqrt to the intrinsicPropagatesPoison list, as these were handled by ConstantFolding but not ValueTracking. The ctpop test is an example of the converse, where it was handled by ValueTracking but not ConstantFolding.
1 parent 25bf90e commit 1d5d125

File tree

5 files changed

+60
-67
lines changed

5 files changed

+60
-67
lines changed

llvm/include/llvm/Analysis/ValueTracking.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,9 @@ LLVM_ABI bool isGuaranteedToExecuteForEveryIteration(const Instruction *I,
727727
/// getGuaranteedNonPoisonOp.
728728
LLVM_ABI bool propagatesPoison(const Use &PoisonOp);
729729

730+
/// Return whether this intrinsic propagates poison for all operands.
731+
LLVM_ABI bool intrinsicPropagatesPoison(Intrinsic::ID IID);
732+
730733
/// Return true if the given instruction must trigger undefined behavior
731734
/// when I is executed with any operands which appear in KnownPoison holding
732735
/// a poison value at the point of execution.

llvm/lib/Analysis/ConstantFolding.cpp

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2229,17 +2229,6 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
22292229
return nullptr;
22302230
}
22312231

2232-
if (isa<PoisonValue>(Operands[0])) {
2233-
// TODO: All of these operations should probably propagate poison.
2234-
switch (IntrinsicID) {
2235-
case Intrinsic::canonicalize:
2236-
case Intrinsic::sqrt:
2237-
return PoisonValue::get(Ty);
2238-
default:
2239-
break;
2240-
}
2241-
}
2242-
22432232
if (isa<UndefValue>(Operands[0])) {
22442233
// cosine(arg) is between -1 and 1. cosine(invalid arg) is NaN.
22452234
// ctpop() is between 0 and bitwidth, pick 0 for undef.
@@ -3228,11 +3217,6 @@ static Constant *ConstantFoldIntrinsicCall2(Intrinsic::ID IntrinsicID, Type *Ty,
32283217
case Intrinsic::smin:
32293218
case Intrinsic::umax:
32303219
case Intrinsic::umin:
3231-
// This is the same as for binary ops - poison propagates.
3232-
// TODO: Poison handling should be consolidated.
3233-
if (isa<PoisonValue>(Operands[0]) || isa<PoisonValue>(Operands[1]))
3234-
return PoisonValue::get(Ty);
3235-
32363220
if (!C0 && !C1)
32373221
return UndefValue::get(Ty);
32383222
if (!C0 || !C1)
@@ -3245,9 +3229,6 @@ static Constant *ConstantFoldIntrinsicCall2(Intrinsic::ID IntrinsicID, Type *Ty,
32453229

32463230
case Intrinsic::scmp:
32473231
case Intrinsic::ucmp:
3248-
if (isa<PoisonValue>(Operands[0]) || isa<PoisonValue>(Operands[1]))
3249-
return PoisonValue::get(Ty);
3250-
32513232
if (!C0 || !C1)
32523233
return ConstantInt::get(Ty, 0);
32533234

@@ -3314,11 +3295,6 @@ static Constant *ConstantFoldIntrinsicCall2(Intrinsic::ID IntrinsicID, Type *Ty,
33143295
}
33153296
case Intrinsic::uadd_sat:
33163297
case Intrinsic::sadd_sat:
3317-
// This is the same as for binary ops - poison propagates.
3318-
// TODO: Poison handling should be consolidated.
3319-
if (isa<PoisonValue>(Operands[0]) || isa<PoisonValue>(Operands[1]))
3320-
return PoisonValue::get(Ty);
3321-
33223298
if (!C0 && !C1)
33233299
return UndefValue::get(Ty);
33243300
if (!C0 || !C1)
@@ -3329,11 +3305,6 @@ static Constant *ConstantFoldIntrinsicCall2(Intrinsic::ID IntrinsicID, Type *Ty,
33293305
return ConstantInt::get(Ty, C0->sadd_sat(*C1));
33303306
case Intrinsic::usub_sat:
33313307
case Intrinsic::ssub_sat:
3332-
// This is the same as for binary ops - poison propagates.
3333-
// TODO: Poison handling should be consolidated.
3334-
if (isa<PoisonValue>(Operands[0]) || isa<PoisonValue>(Operands[1]))
3335-
return PoisonValue::get(Ty);
3336-
33373308
if (!C0 && !C1)
33383309
return UndefValue::get(Ty);
33393310
if (!C0 || !C1)
@@ -3592,11 +3563,6 @@ static Constant *ConstantFoldScalarCall3(StringRef Name,
35923563

35933564
if (IntrinsicID == Intrinsic::smul_fix ||
35943565
IntrinsicID == Intrinsic::smul_fix_sat) {
3595-
// poison * C -> poison
3596-
// C * poison -> poison
3597-
if (isa<PoisonValue>(Operands[0]) || isa<PoisonValue>(Operands[1]))
3598-
return PoisonValue::get(Ty);
3599-
36003566
const APInt *C0, *C1;
36013567
if (!getConstIntOrUndef(Operands[0], C0) ||
36023568
!getConstIntOrUndef(Operands[1], C1))
@@ -3670,6 +3636,11 @@ static Constant *ConstantFoldScalarCall(StringRef Name,
36703636
ArrayRef<Constant *> Operands,
36713637
const TargetLibraryInfo *TLI,
36723638
const CallBase *Call) {
3639+
if (IntrinsicID != Intrinsic::not_intrinsic &&
3640+
any_of(Operands, IsaPred<PoisonValue>) &&
3641+
intrinsicPropagatesPoison(IntrinsicID))
3642+
return PoisonValue::get(Ty);
3643+
36733644
if (Operands.size() == 1)
36743645
return ConstantFoldScalarCall1(Name, IntrinsicID, Ty, Operands, TLI, Call);
36753646

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7879,6 +7879,47 @@ bool llvm::isGuaranteedToExecuteForEveryIteration(const Instruction *I,
78797879
llvm_unreachable("Instruction not contained in its own parent basic block.");
78807880
}
78817881

7882+
bool llvm::intrinsicPropagatesPoison(Intrinsic::ID IID) {
7883+
switch (IID) {
7884+
// TODO: Add more intrinsics.
7885+
case Intrinsic::sadd_with_overflow:
7886+
case Intrinsic::ssub_with_overflow:
7887+
case Intrinsic::smul_with_overflow:
7888+
case Intrinsic::uadd_with_overflow:
7889+
case Intrinsic::usub_with_overflow:
7890+
case Intrinsic::umul_with_overflow:
7891+
// If an input is a vector containing a poison element, the
7892+
// two output vectors (calculated results, overflow bits)'
7893+
// corresponding lanes are poison.
7894+
return true;
7895+
case Intrinsic::ctpop:
7896+
case Intrinsic::ctlz:
7897+
case Intrinsic::cttz:
7898+
case Intrinsic::abs:
7899+
case Intrinsic::smax:
7900+
case Intrinsic::smin:
7901+
case Intrinsic::umax:
7902+
case Intrinsic::umin:
7903+
case Intrinsic::scmp:
7904+
case Intrinsic::ucmp:
7905+
case Intrinsic::bitreverse:
7906+
case Intrinsic::bswap:
7907+
case Intrinsic::sadd_sat:
7908+
case Intrinsic::ssub_sat:
7909+
case Intrinsic::sshl_sat:
7910+
case Intrinsic::uadd_sat:
7911+
case Intrinsic::usub_sat:
7912+
case Intrinsic::ushl_sat:
7913+
case Intrinsic::smul_fix:
7914+
case Intrinsic::smul_fix_sat:
7915+
case Intrinsic::canonicalize:
7916+
case Intrinsic::sqrt:
7917+
return true;
7918+
default:
7919+
return false;
7920+
}
7921+
}
7922+
78827923
bool llvm::propagatesPoison(const Use &PoisonOp) {
78837924
const Operator *I = cast<Operator>(PoisonOp.getUser());
78847925
switch (I->getOpcode()) {
@@ -7889,38 +7930,8 @@ bool llvm::propagatesPoison(const Use &PoisonOp) {
78897930
case Instruction::Select:
78907931
return PoisonOp.getOperandNo() == 0;
78917932
case Instruction::Call:
7892-
if (auto *II = dyn_cast<IntrinsicInst>(I)) {
7893-
switch (II->getIntrinsicID()) {
7894-
// TODO: Add more intrinsics.
7895-
case Intrinsic::sadd_with_overflow:
7896-
case Intrinsic::ssub_with_overflow:
7897-
case Intrinsic::smul_with_overflow:
7898-
case Intrinsic::uadd_with_overflow:
7899-
case Intrinsic::usub_with_overflow:
7900-
case Intrinsic::umul_with_overflow:
7901-
// If an input is a vector containing a poison element, the
7902-
// two output vectors (calculated results, overflow bits)'
7903-
// corresponding lanes are poison.
7904-
return true;
7905-
case Intrinsic::ctpop:
7906-
case Intrinsic::ctlz:
7907-
case Intrinsic::cttz:
7908-
case Intrinsic::abs:
7909-
case Intrinsic::smax:
7910-
case Intrinsic::smin:
7911-
case Intrinsic::umax:
7912-
case Intrinsic::umin:
7913-
case Intrinsic::bitreverse:
7914-
case Intrinsic::bswap:
7915-
case Intrinsic::sadd_sat:
7916-
case Intrinsic::ssub_sat:
7917-
case Intrinsic::sshl_sat:
7918-
case Intrinsic::uadd_sat:
7919-
case Intrinsic::usub_sat:
7920-
case Intrinsic::ushl_sat:
7921-
return true;
7922-
}
7923-
}
7933+
if (auto *II = dyn_cast<IntrinsicInst>(I))
7934+
return intrinsicPropagatesPoison(II->getIntrinsicID());
79247935
return false;
79257936
case Instruction::ICmp:
79267937
case Instruction::FCmp:

llvm/test/Transforms/InstSimplify/fold-intrinsics.ll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,11 @@ define void @powi_i16(float %V, ptr%P) {
4444

4545
ret void
4646
}
47+
48+
define i32 @test_ctpop_poison(i32 %a) {
49+
; CHECK-LABEL: @test_ctpop_poison(
50+
; CHECK-NEXT: ret i32 poison
51+
;
52+
%res = tail call i32 @llvm.ctpop.i32(i32 poison)
53+
ret i32 %res
54+
}

llvm/unittests/Analysis/ValueTrackingTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ TEST(ValueTracking, propagatesPoison) {
910910
{false, "call i32 @llvm.fshr.i32(i32 %x, i32 %y, i32 %shamt)", 0},
911911
{false, "call i32 @llvm.fshr.i32(i32 %x, i32 %y, i32 %shamt)", 1},
912912
{false, "call i32 @llvm.fshr.i32(i32 %x, i32 %y, i32 %shamt)", 2},
913-
{false, "call float @llvm.sqrt.f32(float %fx)", 0},
913+
{true, "call float @llvm.sqrt.f32(float %fx)", 0},
914914
{false, "call float @llvm.powi.f32.i32(float %fx, i32 %x)", 0},
915915
{false, "call float @llvm.sin.f32(float %fx)", 0},
916916
{false, "call float @llvm.cos.f32(float %fx)", 0},

0 commit comments

Comments
 (0)