[llvm] bf23b40 - [ValueTracking] Take poison-generating metadata into account (PR59888)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 20 03:18:41 PST 2023
Author: Nikita Popov
Date: 2023-01-20T12:18:32+01:00
New Revision: bf23b4031eeabfccd46a25ce68414d45ae761304
URL: https://github.com/llvm/llvm-project/commit/bf23b4031eeabfccd46a25ce68414d45ae761304
DIFF: https://github.com/llvm/llvm-project/commit/bf23b4031eeabfccd46a25ce68414d45ae761304.diff
LOG: [ValueTracking] Take poison-generating metadata into account (PR59888)
In canCreateUndefOrPoison(), take not only poison-generating flags,
but also poison-generating metadata into account. The helpers are
written generically, but I believe the only case that can actually
matter is !range on calls -- !nonnull and !align are only valid on
loads, and those can create undef/poison anyway.
Unfortunately, this negatively impacts logical to bitwise and/or
conversion: For ctpop/ctlz/cttz we always attach !range metadata,
which will now block the transform, because it might introduce
poison. It would be possible to recover this regression by supporting
a ConsiderFlagsAndMetadata=false mode in impliesPoison() and clearing
flags/metadata on visited instructions.
Fixes https://github.com/llvm/llvm-project/issues/59888.
Differential Revision: https://reviews.llvm.org/D142115
Added:
Modified:
llvm/include/llvm/Analysis/ValueTracking.h
llvm/include/llvm/IR/Instruction.h
llvm/include/llvm/IR/Operator.h
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/IR/Instruction.cpp
llvm/lib/IR/Operator.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/freeze.ll
llvm/test/Transforms/InstCombine/ispow2.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index 90caf42fe8d3c..d1d31dc795b2a 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -649,16 +649,18 @@ bool programUndefinedIfPoison(const Instruction *Inst);
/// true. If Op raises immediate UB but never creates poison or undef
/// (e.g. sdiv I, 0), canCreatePoison returns false.
///
-/// \p ConsiderFlags controls whether poison producing flags on the
-/// instruction are considered. This can be used to see if the instruction
-/// could still introduce undef or poison even without poison generating flags
-/// which might be on the instruction. (i.e. could the result of
-/// Op->dropPoisonGeneratingFlags() still create poison or undef)
+/// \p ConsiderFlagsAndMetadata controls whether poison producing flags and
+/// metadata on the instruction are considered. This can be used to see if the
+/// instruction could still introduce undef or poison even without poison
+/// generating flags and metadata which might be on the instruction.
+/// (i.e. could the result of Op->dropPoisonGeneratingFlags() still create
+/// poison or undef)
///
/// canCreatePoison returns true if Op can create poison from non-poison
/// operands.
-bool canCreateUndefOrPoison(const Operator *Op, bool ConsiderFlags = true);
-bool canCreatePoison(const Operator *Op, bool ConsiderFlags = true);
+bool canCreateUndefOrPoison(const Operator *Op,
+ bool ConsiderFlagsAndMetadata = true);
+bool canCreatePoison(const Operator *Op, bool ConsiderFlagsAndMetadata = true);
/// Return true if V is poison given that ValAssumedPoison is already poison.
/// For example, if ValAssumedPoison is `icmp X, 10` and V is `icmp X, 5`,
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 5819f766837d1..be25ad954e75a 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -383,6 +383,23 @@ class Instruction : public User,
/// having non-poison inputs.
void dropPoisonGeneratingFlags();
+ /// Return true if this instruction has poison-generating metadata.
+ bool hasPoisonGeneratingMetadata() const LLVM_READONLY;
+
+ /// Drops metadata that may generate poison.
+ void dropPoisonGeneratingMetadata();
+
+ /// Return true if this instruction has poison-generating flags or metadata.
+ bool hasPoisonGeneratingFlagsOrMetadata() const {
+ return hasPoisonGeneratingFlags() || hasPoisonGeneratingMetadata();
+ }
+
+ /// Drops flags and metadata that may generate poison.
+ void dropPoisonGeneratingFlagsAndMetadata() {
+ dropPoisonGeneratingFlags();
+ dropPoisonGeneratingMetadata();
+ }
+
/// This function drops non-debug unknown metadata (through
/// dropUnknownNonDebugMetadata). For calls, it also drops parameter and
/// return attributes that can cause undefined behaviour. Both of these should
diff --git a/llvm/include/llvm/IR/Operator.h b/llvm/include/llvm/IR/Operator.h
index fedf3587e5436..a9da6292e8c77 100644
--- a/llvm/include/llvm/IR/Operator.h
+++ b/llvm/include/llvm/IR/Operator.h
@@ -63,6 +63,10 @@ class Operator : public User {
/// Return true if this operator has flags which may cause this operator
/// to evaluate to poison despite having non-poison inputs.
bool hasPoisonGeneratingFlags() const;
+
+ /// Return true if this operator has poison-generating flags or metadata.
+ /// The latter is only possible for instructions.
+ bool hasPoisonGeneratingFlagsOrMetadata() const;
};
/// Utility class for integer operators which may exhibit overflow - Add, Sub,
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 7c38c6087f3c6..55fd419d51617 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -5236,9 +5236,9 @@ static bool shiftAmountKnownInRange(const Value *ShiftAmount) {
}
static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly,
- bool ConsiderFlags) {
+ bool ConsiderFlagsAndMetadata) {
- if (ConsiderFlags && Op->hasPoisonGeneratingFlags())
+ if (ConsiderFlagsAndMetadata && Op->hasPoisonGeneratingFlagsOrMetadata())
return true;
unsigned Opcode = Op->getOpcode();
@@ -5386,12 +5386,15 @@ static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly,
}
}
-bool llvm::canCreateUndefOrPoison(const Operator *Op, bool ConsiderFlags) {
- return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/false, ConsiderFlags);
+bool llvm::canCreateUndefOrPoison(const Operator *Op,
+ bool ConsiderFlagsAndMetadata) {
+ return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/false,
+ ConsiderFlagsAndMetadata);
}
-bool llvm::canCreatePoison(const Operator *Op, bool ConsiderFlags) {
- return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true, ConsiderFlags);
+bool llvm::canCreatePoison(const Operator *Op, bool ConsiderFlagsAndMetadata) {
+ return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true,
+ ConsiderFlagsAndMetadata);
}
static bool directlyImpliesPoison(const Value *ValAssumedPoison,
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 8e91f0e8929b4..9c88ca17ebde4 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -211,6 +211,18 @@ void Instruction::dropPoisonGeneratingFlags() {
assert(!hasPoisonGeneratingFlags() && "must be kept in sync");
}
+bool Instruction::hasPoisonGeneratingMetadata() const {
+ return hasMetadata(LLVMContext::MD_range) ||
+ hasMetadata(LLVMContext::MD_nonnull) ||
+ hasMetadata(LLVMContext::MD_align);
+}
+
+void Instruction::dropPoisonGeneratingMetadata() {
+ eraseMetadata(LLVMContext::MD_range);
+ eraseMetadata(LLVMContext::MD_nonnull);
+ eraseMetadata(LLVMContext::MD_align);
+}
+
void Instruction::dropUndefImplyingAttrsAndUnknownMetadata(
ArrayRef<unsigned> KnownIDs) {
dropUnknownNonDebugMetadata(KnownIDs);
diff --git a/llvm/lib/IR/Operator.cpp b/llvm/lib/IR/Operator.cpp
index 4e400adbb9ac2..b57f3e3b2967e 100644
--- a/llvm/lib/IR/Operator.cpp
+++ b/llvm/lib/IR/Operator.cpp
@@ -44,6 +44,13 @@ bool Operator::hasPoisonGeneratingFlags() const {
}
}
+bool Operator::hasPoisonGeneratingFlagsOrMetadata() const {
+ if (hasPoisonGeneratingFlags())
+ return true;
+ auto *I = dyn_cast<Instruction>(this);
+ return I && I->hasPoisonGeneratingMetadata();
+}
+
Type *GEPOperator::getSourceElementType() const {
if (auto *I = dyn_cast<GetElementPtrInst>(this))
return I->getSourceElementType();
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index f5b22caa37271..f7f06ed175370 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3829,7 +3829,8 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
// poison. If the only source of new poison is flags, we can simply
// strip them (since we know the only use is the freeze and nothing can
// benefit from them.)
- if (canCreateUndefOrPoison(cast<Operator>(OrigOp), /*ConsiderFlags*/ false))
+ if (canCreateUndefOrPoison(cast<Operator>(OrigOp),
+ /*ConsiderFlagsAndMetadata*/ false))
return nullptr;
// If operand is guaranteed not to be poison, there is no need to add freeze
@@ -3846,7 +3847,7 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
return nullptr;
}
- OrigOpInst->dropPoisonGeneratingFlags();
+ OrigOpInst->dropPoisonGeneratingFlagsAndMetadata();
// If all operands are guaranteed to be non-poison, we can drop freeze.
if (!MaybePoisonOperand)
@@ -3909,7 +3910,7 @@ Instruction *InstCombinerImpl::foldFreezeIntoRecurrence(FreezeInst &FI,
Instruction *I = dyn_cast<Instruction>(V);
if (!I || canCreateUndefOrPoison(cast<Operator>(I),
- /*ConsiderFlags*/ false))
+ /*ConsiderFlagsAndMetadata*/ false))
return nullptr;
DropFlags.push_back(I);
@@ -3917,7 +3918,7 @@ Instruction *InstCombinerImpl::foldFreezeIntoRecurrence(FreezeInst &FI,
}
for (Instruction *I : DropFlags)
- I->dropPoisonGeneratingFlags();
+ I->dropPoisonGeneratingFlagsAndMetadata();
if (StartNeedsFreeze) {
Builder.SetInsertPoint(StartBB->getTerminator());
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index 21a89d873b92b..7bbc32f277398 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -1125,5 +1125,5 @@ define i32 @freeze_ctpop(i32 %x) {
; CHECK: [[META0:![0-9]+]] = !{}
; CHECK: [[META1:![0-9]+]] = !{i64 4}
; CHECK: [[RNG2]] = !{i32 0, i32 100}
-; CHECK: [[RNG3]] = !{i32 0, i32 12}
+; CHECK: [[RNG3]] = !{i32 0, i32 33}
;.
diff --git a/llvm/test/Transforms/InstCombine/ispow2.ll b/llvm/test/Transforms/InstCombine/ispow2.ll
index c9ffd96e39383..a05f9d87c9270 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
More information about the llvm-commits
mailing list