[llvm] [InstSimplify] Guard and/or implied-cond folds against poison (PR #211534)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 23 05:20:31 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Akshat Dalal (adalal-2441)
<details>
<summary>Changes</summary>
When one operand of an and/or implies the other, InstSimplify may replace the whole expression with a single operand. This is wrong when the kept operand can be poison but the dropped one absorbs it.
Example:
`%a = icmp slt i32 %x, 0`
`%b = icmp samesign ult i32 %x, -2147483647`
`%c = and i1 %a, %b ; folded to %b`
For %x = 2, %a is false and %b is poison (samesign is violated), so the and is a well-defined false. But the fold rewrites %c to %b, turning that defined false into poison.
Fix: only apply the fold when the kept operand being poison guarantees the original and/or was already poison. This is checked by a new helper, structurallyImpliesPoison(), which looks only at how the values are defined (not their uses), so it stays correct even though the fold removes a use.
NOTE: We cannot use the existing impliesPoison API. It relies on isGuaranteedNotToBePoison, which decides a value isn't poison by looking at how it's used (dominating conditions, assumes, etc.). But this fold removes a use, so that reasoning can be undone by the fold itself - unsound, and unstable as the IR changes. structurallyImpliesPoison uses similar logic but only looks at how a value is defined, so its answer holds no matter how uses change.
---
Full diff: https://github.com/llvm/llvm-project/pull/211534.diff
5 Files Affected:
- (modified) llvm/include/llvm/Analysis/ValueTracking.h (+8)
- (modified) llvm/lib/Analysis/InstructionSimplify.cpp (+16-4)
- (modified) llvm/lib/Analysis/ValueTracking.cpp (+39)
- (added) llvm/test/Transforms/InstSimplify/and-or-implied-cond-samesign-poison.ll (+78)
- (modified) llvm/test/Transforms/InstSimplify/implies.ll (+6-2)
``````````diff
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index c87e39bca8215..4c5bdbaa0bfec 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -811,6 +811,14 @@ LLVM_ABI bool canCreatePoison(const Operator *Op,
/// impliesPoison returns true.
LLVM_ABI bool impliesPoison(const Value *ValAssumedPoison, const Value *V);
+/// Return true if V is guaranteed to be poison whenever ValAssumedPoison is
+/// poison, based only on how the two values are defined, not on how they are
+/// used. Their uses are ignored on purpose: this is called to justify removing
+/// a use, so any conclusion drawn from the current uses could be invalidated by
+/// the transform it enables.
+LLVM_ABI bool structurallyImpliesPoison(const Value *ValAssumedPoison,
+ const Value *V);
+
/// Return true if this function can prove that V does not have undef bits
/// and is never poison. If V is an aggregate value or vector, check whether
/// all elements (except padding) are not undef or poison.
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index ddaa44f43e631..cc6129c331410 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -2225,7 +2225,10 @@ static Value *simplifyAndInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
if (Op0->getType()->isIntOrIntVectorTy(1)) {
if (std::optional<bool> Implied = isImpliedCondition(Op0, Op1, Q.DL)) {
// If Op0 is true implies Op1 is true, then Op0 is a subset of Op1.
- if (*Implied == true)
+ // Only refine to Op0 if Op0 being poison implies the 'and' was already
+ // poison; otherwise (Op1 == false absorbing a poison Op0) we would turn
+ // a well-defined result into poison.
+ if (*Implied == true && structurallyImpliesPoison(Op0, Op1))
return Op0;
// If Op0 is true implies Op1 is false, then they are not true together.
if (*Implied == false)
@@ -2233,7 +2236,10 @@ static Value *simplifyAndInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
}
if (std::optional<bool> Implied = isImpliedCondition(Op1, Op0, Q.DL)) {
// If Op1 is true implies Op0 is true, then Op1 is a subset of Op0.
- if (*Implied)
+ // Only refine to Op1 if Op1 being poison implies the 'and' was already
+ // poison; otherwise (Op0 == false absorbing a poison Op1) we would turn
+ // a well-defined result into poison.
+ if (*Implied && structurallyImpliesPoison(Op1, Op0))
return Op1;
// If Op1 is true implies Op0 is false, then they are not true together.
if (!*Implied)
@@ -2498,7 +2504,10 @@ static Value *simplifyOrInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
if (std::optional<bool> Implied =
isImpliedCondition(Op0, Op1, Q.DL, false)) {
// If Op0 is false implies Op1 is false, then Op1 is a subset of Op0.
- if (*Implied == false)
+ // Only refine to Op0 if Op0 being poison implies the 'or' was already
+ // poison; otherwise (Op1 == true absorbing a poison Op0) we would turn
+ // a well-defined result into poison.
+ if (*Implied == false && structurallyImpliesPoison(Op0, Op1))
return Op0;
// If Op0 is false implies Op1 is true, then at least one is always true.
if (*Implied == true)
@@ -2507,7 +2516,10 @@ static Value *simplifyOrInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
if (std::optional<bool> Implied =
isImpliedCondition(Op1, Op0, Q.DL, false)) {
// If Op1 is false implies Op0 is false, then Op0 is a subset of Op1.
- if (*Implied == false)
+ // Only refine to Op1 if Op1 being poison implies the 'or' was already
+ // poison; otherwise (Op0 == true absorbing a poison Op1) we would turn
+ // a well-defined result into poison.
+ if (*Implied == false && structurallyImpliesPoison(Op1, Op0))
return Op1;
// If Op1 is false implies Op0 is true, then at least one is always true.
if (*Implied == true)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index efc14f9a639da..c68cd439fc4da 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -7895,6 +7895,45 @@ bool llvm::impliesPoison(const Value *ValAssumedPoison, const Value *V) {
return ::impliesPoison(ValAssumedPoison, V, /* Depth */ 0);
}
+// Return true if V can't be poison, looking only at its structure and not its
+// use-list.
+static bool isDefinitionGuaranteedNotToBePoison(const Value *V) {
+ if (const auto *C = dyn_cast<Constant>(V))
+ return !isa<ConstantExpr>(C) && !isa<PoisonValue>(C) &&
+ !C->containsPoisonElement();
+ if (const auto *Arg = dyn_cast<Argument>(V))
+ return Arg->hasAttribute(Attribute::NoUndef);
+ return isa<FreezeInst>(V);
+}
+
+static bool structurallyImpliesPoison(const Value *ValAssumedPoison,
+ const Value *V, unsigned Depth) {
+ if (isDefinitionGuaranteedNotToBePoison(ValAssumedPoison))
+ return true;
+
+ if (directlyImpliesPoison(ValAssumedPoison, V, /* Depth */ 0))
+ return true;
+
+ const unsigned MaxDepth = 2;
+ if (Depth >= MaxDepth)
+ return false;
+
+ // ValAssumedPoison can't create poison locally, so its poison comes from an
+ // operand; require each operand to imply V's poison.
+ const auto *I = dyn_cast<Instruction>(ValAssumedPoison);
+ if (I && !canCreatePoison(cast<Operator>(I))) {
+ return all_of(I->operands(), [=](const Value *Op) {
+ return structurallyImpliesPoison(Op, V, Depth + 1);
+ });
+ }
+ return false;
+}
+
+bool llvm::structurallyImpliesPoison(const Value *ValAssumedPoison,
+ const Value *V) {
+ return ::structurallyImpliesPoison(ValAssumedPoison, V, /* Depth */ 0);
+}
+
static bool programUndefinedIfUndefOrPoison(const Value *V, bool PoisonOnly);
static bool isGuaranteedNotToBeUndefOrPoison(
diff --git a/llvm/test/Transforms/InstSimplify/and-or-implied-cond-samesign-poison.ll b/llvm/test/Transforms/InstSimplify/and-or-implied-cond-samesign-poison.ll
new file mode 100644
index 0000000000000..fd40eec3469c2
--- /dev/null
+++ b/llvm/test/Transforms/InstSimplify/and-or-implied-cond-samesign-poison.ll
@@ -0,0 +1,78 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=instsimplify < %s | FileCheck %s
+
+; Do not refine and/or of icmps when one arm can be poison but the other
+; absorbs it (and i1 false, poison = false; or i1 true, poison = true).
+; %c feeds a branch, and branching on poison is UB, so a use-list check
+; (impliesPoison) deduces %b is never poison and wrongly folds %c to %b. The
+; structural check (structurallyImpliesPoison) ignores uses and keeps %c.
+
+define ptr @ssub_sat_minus_one(ptr %p, i32 %x) {
+; CHECK-LABEL: @ssub_sat_minus_one(
+; CHECK-NEXT: [[A:%.*]] = icmp slt i32 [[X:%.*]], 0
+; CHECK-NEXT: [[B:%.*]] = icmp samesign ult i32 [[X]], -2147483647
+; CHECK-NEXT: [[C:%.*]] = and i1 [[A]], [[B]]
+; CHECK-NEXT: [[D:%.*]] = add nsw i32 [[X]], -1
+; CHECK-NEXT: [[R:%.*]] = select i1 [[C]], i32 -2147483648, i32 [[D]]
+; CHECK-NEXT: [[T:%.*]] = trunc i32 [[R]] to i8
+; CHECK-NEXT: store i8 [[T]], ptr [[P:%.*]], align 1
+; CHECK-NEXT: [[M:%.*]] = and i32 [[R]], 1
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[M]], 0
+; CHECK-NEXT: br i1 [[CMP]], label [[LOOP:%.*]], label [[OK:%.*]]
+; CHECK: ok:
+; CHECK-NEXT: ret ptr null
+; CHECK: loop:
+; CHECK-NEXT: br label [[LOOP]]
+;
+ %a = icmp slt i32 %x, 0
+ %b = icmp samesign ult i32 %x, -2147483647
+ %c = and i1 %a, %b
+ %d = add nsw i32 %x, -1
+ %r = select i1 %c, i32 -2147483648, i32 %d
+ %t = trunc i32 %r to i8
+ store i8 %t, ptr %p, align 1
+ %m = and i32 %r, 1
+ %cmp = icmp eq i32 %m, 0
+ br i1 %cmp, label %loop, label %ok
+
+ok:
+ ret ptr null
+
+loop:
+ br label %loop
+}
+
+define ptr @sadd_sat_plus_one(ptr %p, i32 %x) {
+; CHECK-LABEL: @sadd_sat_plus_one(
+; CHECK-NEXT: [[A:%.*]] = icmp sgt i32 [[X:%.*]], 0
+; CHECK-NEXT: [[B:%.*]] = icmp samesign slt i32 [[X]], -2147483647
+; CHECK-NEXT: [[C:%.*]] = or i1 [[A]], [[B]]
+; CHECK-NEXT: [[D:%.*]] = add nsw i32 [[X]], 1
+; CHECK-NEXT: [[R:%.*]] = select i1 [[C]], i32 [[D]], i32 -2147483648
+; CHECK-NEXT: [[T:%.*]] = trunc i32 [[R]] to i8
+; CHECK-NEXT: store i8 [[T]], ptr [[P:%.*]], align 1
+; CHECK-NEXT: [[M:%.*]] = and i32 [[R]], 1
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[M]], 0
+; CHECK-NEXT: br i1 [[CMP]], label [[LOOP:%.*]], label [[OK:%.*]]
+; CHECK: ok:
+; CHECK-NEXT: ret ptr null
+; CHECK: loop:
+; CHECK-NEXT: br label [[LOOP]]
+;
+ %a = icmp sgt i32 %x, 0
+ %b = icmp samesign slt i32 %x, -2147483647
+ %c = or i1 %a, %b
+ %d = add nsw i32 %x, 1
+ %r = select i1 %c, i32 %d, i32 -2147483648
+ %t = trunc i32 %r to i8
+ store i8 %t, ptr %p, align 1
+ %m = and i32 %r, 1
+ %cmp = icmp eq i32 %m, 0
+ br i1 %cmp, label %loop, label %ok
+
+ok:
+ ret ptr null
+
+loop:
+ br label %loop
+}
diff --git a/llvm/test/Transforms/InstSimplify/implies.ll b/llvm/test/Transforms/InstSimplify/implies.ll
index 53d8d79add301..84ff9153ae568 100644
--- a/llvm/test/Transforms/InstSimplify/implies.ll
+++ b/llvm/test/Transforms/InstSimplify/implies.ll
@@ -359,7 +359,9 @@ define i1 @pr70374(i32 %x, i32 %y, i32 %z) {
; CHECK-LABEL: @pr70374(
; CHECK-NEXT: [[ADD:%.*]] = add nuw i32 [[Y:%.*]], [[Z:%.*]]
; CHECK-NEXT: [[CMP1:%.*]] = icmp ule i32 [[ADD]], [[X:%.*]]
-; CHECK-NEXT: ret i1 [[CMP1]]
+; CHECK-NEXT: [[CMP2:%.*]] = icmp uge i32 [[X]], [[Y]]
+; CHECK-NEXT: [[RES:%.*]] = and i1 [[CMP2]], [[CMP1]]
+; CHECK-NEXT: ret i1 [[RES]]
;
%add = add nuw i32 %y, %z
%cmp1 = icmp ule i32 %add, %x
@@ -372,7 +374,9 @@ define i1 @pr70374_commuted_add(i32 %x, i32 %y, i32 %z) {
; CHECK-LABEL: @pr70374_commuted_add(
; CHECK-NEXT: [[ADD:%.*]] = add nuw i32 [[Z:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[CMP1:%.*]] = icmp ule i32 [[ADD]], [[X:%.*]]
-; CHECK-NEXT: ret i1 [[CMP1]]
+; CHECK-NEXT: [[CMP2:%.*]] = icmp uge i32 [[X]], [[Y]]
+; CHECK-NEXT: [[RES:%.*]] = and i1 [[CMP2]], [[CMP1]]
+; CHECK-NEXT: ret i1 [[RES]]
;
%add = add nuw i32 %z, %y
%cmp1 = icmp ule i32 %add, %x
``````````
</details>
https://github.com/llvm/llvm-project/pull/211534
More information about the llvm-commits
mailing list