[llvm] cd2fc73 - Revert "[ValueTracking][InstCombine] Add a new API to allow to ignore poison generating flags or metadatas when implying poison"
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Mon May 29 07:45:07 PDT 2023
Author: Florian Hahn
Date: 2023-05-29T15:44:37+01:00
New Revision: cd2fc73b49851540b06f91e89a42bdc5affa7e49
URL: https://github.com/llvm/llvm-project/commit/cd2fc73b49851540b06f91e89a42bdc5affa7e49
DIFF: https://github.com/llvm/llvm-project/commit/cd2fc73b49851540b06f91e89a42bdc5affa7e49.diff
LOG: Revert "[ValueTracking][InstCombine] Add a new API to allow to ignore poison generating flags or metadatas when implying poison"
This reverts commit 754f3ae65518331b7175d7a9b4a124523ebe6eac.
Unfortunately the change can cause regressions due to dropping flags
from instructions (like nuw,nsw,inbounds), prevent further optimizations
depending on those flags.
A simple example is the IR below, where `inbounds` is dropped with the
patch and the phase-ordering test added in 7c91d82ab912fae8b.
define i1 @test(ptr %base, i64 noundef %len, ptr %p2) {
bb:
%gep = getelementptr inbounds i32, ptr %base, i64 %len
%c.1 = icmp uge ptr %p2, %base
%c.2 = icmp ult ptr %p2, %gep
%select = select i1 %c.1, i1 %c.2, i1 false
ret i1 %select
}
For more discussion, see D149404.
Added:
Modified:
llvm/include/llvm/Analysis/ValueTracking.h
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
llvm/test/Transforms/InstCombine/ispow2.ll
llvm/test/Transforms/InstCombine/prevent-cmp-merge.ll
llvm/test/Transforms/PhaseOrdering/iterator-with-runtime-check.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index 4132654ac94d0..48bd1ee228b9f 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -946,13 +946,6 @@ bool canCreatePoison(const Operator *Op, bool ConsiderFlagsAndMetadata = true);
/// impliesPoison returns true.
bool impliesPoison(const Value *ValAssumedPoison, const Value *V);
-/// Return true if V is poison given that ValAssumedPoison is already poison.
-/// Poison generating flags or metadata are ignored in the process of implying.
-/// And the ignored instructions will be recorded in IgnoredInsts.
-bool impliesPoisonIgnoreFlagsOrMetadata(
- Value *ValAssumedPoison, const Value *V,
- SmallVectorImpl<Instruction *> &IgnoredInsts);
-
/// 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/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 7ec34cdca0be5..fc15fb8c02726 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6599,9 +6599,8 @@ static bool directlyImpliesPoison(const Value *ValAssumedPoison,
return false;
}
-static bool
-impliesPoison(Value *ValAssumedPoison, const Value *V, unsigned Depth,
- SmallVectorImpl<Instruction *> *IgnoredInsts = nullptr) {
+static bool impliesPoison(const Value *ValAssumedPoison, const Value *V,
+ unsigned Depth) {
if (isGuaranteedNotToBePoison(ValAssumedPoison))
return true;
@@ -6612,30 +6611,17 @@ impliesPoison(Value *ValAssumedPoison, const Value *V, unsigned Depth,
if (Depth >= MaxDepth)
return false;
- auto *I = dyn_cast<Instruction>(ValAssumedPoison);
- if (!I || canCreatePoison(cast<Operator>(I),
- /*ConsiderFlagsAndMetadata*/ !IgnoredInsts))
- return false;
-
- for (Value *Op : I->operands())
- if (!impliesPoison(Op, V, Depth + 1, IgnoredInsts))
- return false;
-
- if (IgnoredInsts && I->hasPoisonGeneratingFlagsOrMetadata())
- IgnoredInsts->push_back(I);
-
- return true;
+ const auto *I = dyn_cast<Instruction>(ValAssumedPoison);
+ if (I && !canCreatePoison(cast<Operator>(I))) {
+ return all_of(I->operands(), [=](const Value *Op) {
+ return impliesPoison(Op, V, Depth + 1);
+ });
+ }
+ return false;
}
bool llvm::impliesPoison(const Value *ValAssumedPoison, const Value *V) {
- return ::impliesPoison(const_cast<Value *>(ValAssumedPoison), V,
- /* Depth */ 0);
-}
-
-bool llvm::impliesPoisonIgnoreFlagsOrMetadata(
- Value *ValAssumedPoison, const Value *V,
- SmallVectorImpl<Instruction *> &IgnoredInsts) {
- return ::impliesPoison(ValAssumedPoison, V, /* Depth */ 0, &IgnoredInsts);
+ return ::impliesPoison(ValAssumedPoison, V, /* Depth */ 0);
}
static bool programUndefinedIfUndefOrPoison(const Value *V,
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 1b29304338092..32b3c56dc9a21 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2924,32 +2924,21 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
auto *Zero = ConstantInt::getFalse(SelType);
Value *A, *B, *C, *D;
- auto dropPoisonGeneratingFlagsAndMetadata =
- [](ArrayRef<Instruction *> Insts) {
- for (auto *I : Insts)
- I->dropPoisonGeneratingFlagsAndMetadata();
- };
// Folding select to and/or i1 isn't poison safe in general. impliesPoison
// checks whether folding it does not convert a well-defined value into
// poison.
if (match(TrueVal, m_One())) {
+ if (impliesPoison(FalseVal, CondVal)) {
+ // Change: A = select B, true, C --> A = or B, C
+ return BinaryOperator::CreateOr(CondVal, FalseVal);
+ }
+
if (auto *LHS = dyn_cast<FCmpInst>(CondVal))
if (auto *RHS = dyn_cast<FCmpInst>(FalseVal))
if (Value *V = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ false,
/*IsSelectLogical*/ true))
return replaceInstUsesWith(SI, V);
- // Some patterns can be matched by both of the above and following
- // combinations. Because we need to drop poison generating
- // flags and metadatas for the following combination, it has less priority
- // than the above combination.
- SmallVector<Instruction *> IgnoredInsts;
- if (impliesPoisonIgnoreFlagsOrMetadata(FalseVal, CondVal, IgnoredInsts)) {
- dropPoisonGeneratingFlagsAndMetadata(IgnoredInsts);
- // Change: A = select B, true, C --> A = or B, C
- return BinaryOperator::CreateOr(CondVal, FalseVal);
- }
-
// (A && B) || (C && B) --> (A || C) && B
if (match(CondVal, m_LogicalAnd(m_Value(A), m_Value(B))) &&
match(FalseVal, m_LogicalAnd(m_Value(C), m_Value(D))) &&
@@ -2980,23 +2969,17 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
}
if (match(FalseVal, m_Zero())) {
+ if (impliesPoison(TrueVal, CondVal)) {
+ // Change: A = select B, C, false --> A = and B, C
+ return BinaryOperator::CreateAnd(CondVal, TrueVal);
+ }
+
if (auto *LHS = dyn_cast<FCmpInst>(CondVal))
if (auto *RHS = dyn_cast<FCmpInst>(TrueVal))
if (Value *V = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ true,
/*IsSelectLogical*/ true))
return replaceInstUsesWith(SI, V);
- // Some patterns can be matched by both of the above and following
- // combinations. Because we need to drop poison generating
- // flags and metadatas for the following combination, it has less priority
- // than the above combination.
- SmallVector<Instruction *> IgnoredInsts;
- if (impliesPoisonIgnoreFlagsOrMetadata(TrueVal, CondVal, IgnoredInsts)) {
- dropPoisonGeneratingFlagsAndMetadata(IgnoredInsts);
- // Change: A = select B, C, false --> A = and B, C
- return BinaryOperator::CreateAnd(CondVal, TrueVal);
- }
-
// (A || B) && (C || B) --> (A && C) || B
if (match(CondVal, m_LogicalOr(m_Value(A), m_Value(B))) &&
match(TrueVal, m_LogicalOr(m_Value(C), m_Value(D))) &&
diff --git a/llvm/test/Transforms/InstCombine/ispow2.ll b/llvm/test/Transforms/InstCombine/ispow2.ll
index ce178c29bba2a..191ff9f005a5d 100644
--- a/llvm/test/Transforms/InstCombine/ispow2.ll
+++ b/llvm/test/Transforms/InstCombine/ispow2.ll
@@ -282,7 +282,7 @@ define i1 @is_pow2_ctpop_wrong_cmp_op1_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[T0]], 3
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -314,7 +314,7 @@ define i1 @is_pow2_ctpop_wrong_cmp_op2_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[T0]], 2
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 1
-; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -346,7 +346,7 @@ define i1 @is_pow2_ctpop_wrong_pred1_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 [[T0]], 2
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -378,7 +378,7 @@ define i1 @is_pow2_ctpop_wrong_pred2_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[T0]], 2
; CHECK-NEXT: [[CMP2:%.*]] = icmp sgt i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = and i1 [[CMP2]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP2]], i1 [[CMP]], i1 false
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -493,7 +493,7 @@ define i1 @isnot_pow2_ctpop_wrong_cmp_op1_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 [[T0]], 2
; CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[ISZERO]], i1 true, i1 [[CMP]]
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -525,7 +525,7 @@ define i1 @isnot_pow2_ctpop_wrong_cmp_op2_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 [[T0]], 1
; CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i32 [[X]], 1
-; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[ISZERO]], i1 true, i1 [[CMP]]
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -557,7 +557,7 @@ define i1 @isnot_pow2_ctpop_wrong_pred2_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 [[T0]], 1
; CHECK-NEXT: [[CMP2:%.*]] = icmp slt i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = or i1 [[CMP2]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP2]], i1 true, i1 [[CMP]]
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -855,7 +855,7 @@ define i1 @is_pow2or0_ctpop_wrong_cmp_op1_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[T0]], 3
; CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[ISZERO]], i1 true, i1 [[CMP]]
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -914,11 +914,7 @@ define i1 @is_pow2or0_ctpop_wrong_pred2(i32 %x) {
define i1 @is_pow2or0_ctpop_wrong_pred2_logical(i32 %x) {
; CHECK-LABEL: @is_pow2or0_ctpop_wrong_pred2_logical(
-; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT: [[ISZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT: ret i1 [[R]]
+; CHECK-NEXT: ret i1 true
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
%cmp = icmp ne i32 %t0, 1
@@ -1062,7 +1058,7 @@ define i1 @isnot_pow2nor0_ctpop_wrong_cmp_op1_logical(i32 %x) {
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[T0]], 5
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
+; CHECK-NEXT: [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
; CHECK-NEXT: ret i1 [[R]]
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
@@ -1121,11 +1117,7 @@ define i1 @isnot_pow2nor0_ctpop_wrong_pred2(i32 %x) {
define i1 @isnot_pow2nor0_ctpop_wrong_pred2_logical(i32 %x) {
; CHECK-LABEL: @isnot_pow2nor0_ctpop_wrong_pred2_logical(
-; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT: [[NOTZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
-; CHECK-NEXT: ret i1 [[R]]
+; CHECK-NEXT: ret i1 false
;
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
%cmp = icmp eq i32 %t0, 1
diff --git a/llvm/test/Transforms/InstCombine/prevent-cmp-merge.ll b/llvm/test/Transforms/InstCombine/prevent-cmp-merge.ll
index a24ae9b9c57b9..cd05022b0d35d 100644
--- a/llvm/test/Transforms/InstCombine/prevent-cmp-merge.ll
+++ b/llvm/test/Transforms/InstCombine/prevent-cmp-merge.ll
@@ -71,10 +71,10 @@ define zeroext i1 @test3(i32 %lhs, i32 %rhs) {
define zeroext i1 @test3_logical(i32 %lhs, i32 %rhs) {
; CHECK-LABEL: @test3_logical(
-; CHECK-NEXT: [[SUB:%.*]] = sub i32 [[LHS:%.*]], [[RHS:%.*]]
+; CHECK-NEXT: [[SUB:%.*]] = sub nsw i32 [[LHS:%.*]], [[RHS:%.*]]
; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 [[LHS]], [[RHS]]
; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 [[SUB]], 31
-; CHECK-NEXT: [[SEL:%.*]] = or i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i1 true, i1 [[CMP2]]
; CHECK-NEXT: ret i1 [[SEL]]
;
diff --git a/llvm/test/Transforms/PhaseOrdering/iterator-with-runtime-check.ll b/llvm/test/Transforms/PhaseOrdering/iterator-with-runtime-check.ll
index ebe507d8e9c35..23b1b2b3cd87d 100644
--- a/llvm/test/Transforms/PhaseOrdering/iterator-with-runtime-check.ll
+++ b/llvm/test/Transforms/PhaseOrdering/iterator-with-runtime-check.ll
@@ -24,12 +24,11 @@ define void @test_fill_with_foreach([2 x i64] %elems.coerce) {
; CHECK-NEXT: [[ELEMS_COERCE_FCA_0_EXTRACT:%.*]] = extractvalue [2 x i64] [[ELEMS_COERCE]], 0
; CHECK-NEXT: [[TMP0:%.*]] = inttoptr i64 [[ELEMS_COERCE_FCA_0_EXTRACT]] to ptr
; CHECK-NEXT: [[ELEMS_COERCE_FCA_1_EXTRACT:%.*]] = extractvalue [2 x i64] [[ELEMS_COERCE]], 1
-; CHECK-NEXT: [[ADD_PTR_I:%.*]] = getelementptr i32, ptr [[TMP0]], i64 [[ELEMS_COERCE_FCA_1_EXTRACT]]
+; CHECK-NEXT: [[ADD_PTR_I:%.*]] = getelementptr inbounds i32, ptr [[TMP0]], i64 [[ELEMS_COERCE_FCA_1_EXTRACT]]
; CHECK-NEXT: [[CMP_NOT_I_I_I_I:%.*]] = icmp slt i64 [[ELEMS_COERCE_FCA_1_EXTRACT]], 0
; CHECK-NEXT: br i1 [[CMP_NOT_I_I_I_I]], label [[ERROR:%.*]], label [[FOR_COND_PREHEADER:%.*]]
; CHECK: for.cond.preheader:
-; CHECK-NEXT: [[ADD_PTR_I_IDX_MASK:%.*]] = and i64 [[ELEMS_COERCE_FCA_1_EXTRACT]], 4611686018427387903
-; CHECK-NEXT: [[CMP_I_NOT2:%.*]] = icmp eq i64 [[ADD_PTR_I_IDX_MASK]], 0
+; CHECK-NEXT: [[CMP_I_NOT2:%.*]] = icmp eq i64 [[ELEMS_COERCE_FCA_1_EXTRACT]], 0
; CHECK-NEXT: br i1 [[CMP_I_NOT2]], label [[COMMON_RET:%.*]], label [[FOR_BODY:%.*]]
; CHECK: common.ret:
; CHECK-NEXT: ret void
More information about the llvm-commits
mailing list