[llvm] [ValueTracking] Make `computeKnownBitsFromContext` deterministic (PR #75452)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 14 01:45:14 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-analysis
Author: Yingwei Zheng (dtcxzyw)
<details>
<summary>Changes</summary>
Fixes #<!-- -->75369.
The latter `getFreelyInverted` failed because the former call increased the size of the basic block, and we reached the scan limit in `isValidAssumeForContext`.
https://github.com/llvm/llvm-project/blob/8cb842879d44ba7b5c1d3e158b8824cae1db6092/llvm/lib/Analysis/ValueTracking.cpp#L509-L517
This patch adds an option `Deterministic` to `SimplifyQuery`. If `Deterministic` is true, we guarantee that the result of the query is deterministic during instruction construction.
---
Full diff: https://github.com/llvm/llvm-project/pull/75452.diff
5 Files Affected:
- (modified) llvm/include/llvm/Analysis/SimplifyQuery.h (+14-4)
- (modified) llvm/include/llvm/Analysis/ValueTracking.h (+4-1)
- (modified) llvm/lib/Analysis/ValueTracking.cpp (+11-9)
- (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+3-2)
- (added) llvm/test/Transforms/InstCombine/pr75369.ll (+36)
``````````diff
diff --git a/llvm/include/llvm/Analysis/SimplifyQuery.h b/llvm/include/llvm/Analysis/SimplifyQuery.h
index e5e6ae0d3d8e3e..51ddbe1f0bd25f 100644
--- a/llvm/include/llvm/Analysis/SimplifyQuery.h
+++ b/llvm/include/llvm/Analysis/SimplifyQuery.h
@@ -75,6 +75,10 @@ struct SimplifyQuery {
/// allowed to assume a particular value for a use of undef for example.
bool CanUseUndef = true;
+ /// Controls whether the result of query is guaranteed to be deterministic.
+ /// If it is false, the result may change during instruction construction.
+ bool Deterministic = false;
+
SimplifyQuery(const DataLayout &DL, const Instruction *CXTI = nullptr)
: DL(DL), CxtI(CXTI) {}
@@ -82,22 +86,28 @@ struct SimplifyQuery {
const DominatorTree *DT = nullptr,
AssumptionCache *AC = nullptr,
const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
- bool CanUseUndef = true, const DomConditionCache *DC = nullptr)
+ bool CanUseUndef = true, const DomConditionCache *DC = nullptr,
+ bool Deterministic = false)
: DL(DL), TLI(TLI), DT(DT), AC(AC), CxtI(CXTI), DC(DC), IIQ(UseInstrInfo),
- CanUseUndef(CanUseUndef) {}
+ CanUseUndef(CanUseUndef), Deterministic(Deterministic) {}
SimplifyQuery(const DataLayout &DL, const DominatorTree *DT,
AssumptionCache *AC = nullptr,
const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
- bool CanUseUndef = true)
+ bool CanUseUndef = true, bool Deterministic = false)
: DL(DL), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo),
- CanUseUndef(CanUseUndef) {}
+ CanUseUndef(CanUseUndef), Deterministic(Deterministic) {}
SimplifyQuery getWithInstruction(const Instruction *I) const {
SimplifyQuery Copy(*this);
Copy.CxtI = I;
return Copy;
}
+ SimplifyQuery getWithDeterministic() const {
+ SimplifyQuery Copy(*this);
+ Copy.Deterministic = true;
+ return Copy;
+ }
SimplifyQuery getWithoutUndef() const {
SimplifyQuery Copy(*this);
Copy.CanUseUndef = false;
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index a3186e61b94adf..bf155406d554a6 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -811,8 +811,11 @@ bool isAssumeLikeIntrinsic(const Instruction *I);
/// Return true if it is valid to use the assumptions provided by an
/// assume intrinsic, I, at the point in the control-flow identified by the
/// context instruction, CxtI.
+/// If \p Deterministic is true, the result is guaranteed to be deterministic
+/// regardless of the size of a basic block.
bool isValidAssumeForContext(const Instruction *I, const Instruction *CxtI,
- const DominatorTree *DT = nullptr);
+ const DominatorTree *DT = nullptr,
+ bool Deterministic = false);
enum class OverflowResult {
/// Always overflows in the direction of signed/unsigned min value.
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 5445746ab2a1bc..f0ae6a71020382 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -485,7 +485,8 @@ bool llvm::isAssumeLikeIntrinsic(const Instruction *I) {
bool llvm::isValidAssumeForContext(const Instruction *Inv,
const Instruction *CxtI,
- const DominatorTree *DT) {
+ const DominatorTree *DT,
+ bool Deterministic) {
// There are two restrictions on the use of an assume:
// 1. The assume must dominate the context (or the control flow must
// reach the assume whenever it reaches the context).
@@ -513,7 +514,7 @@ bool llvm::isValidAssumeForContext(const Instruction *Inv,
// to avoid a compile-time explosion. This limit is chosen arbitrarily, so
// it can be adjusted if needed (could be turned into a cl::opt).
auto Range = make_range(CxtI->getIterator(), Inv->getIterator());
- if (!isGuaranteedToTransferExecutionToSuccessor(Range, 15))
+ if (Deterministic || !isGuaranteedToTransferExecutionToSuccessor(Range, 15))
return false;
return !isEphemeralValueOf(Inv, CxtI);
@@ -591,7 +592,7 @@ static bool isKnownNonZeroFromAssume(const Value *V, const SimplifyQuery &Q) {
(RK.AttrKind == Attribute::Dereferenceable &&
!NullPointerIsDefined(Q.CxtI->getFunction(),
V->getType()->getPointerAddressSpace()))) &&
- isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic))
return true;
}
continue;
@@ -607,7 +608,8 @@ static bool isKnownNonZeroFromAssume(const Value *V, const SimplifyQuery &Q) {
if (!match(I->getArgOperand(0), m_c_ICmp(Pred, m_V, m_Value(RHS))))
return false;
- if (cmpExcludesZero(Pred, RHS) && isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ if (cmpExcludesZero(Pred, RHS) &&
+ isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic))
return true;
}
@@ -756,7 +758,7 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
*I, I->bundle_op_info_begin()[Elem.Index])) {
if (RK.WasOn == V && RK.AttrKind == Attribute::Alignment &&
isPowerOf2_64(RK.ArgValue) &&
- isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic))
Known.Zero.setLowBits(Log2_64(RK.ArgValue));
}
continue;
@@ -768,14 +770,14 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
Value *Arg = I->getArgOperand(0);
- if (Arg == V && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+ if (Arg == V && isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic)) {
assert(BitWidth == 1 && "assume operand is not i1?");
(void)BitWidth;
Known.setAllOnes();
return;
}
if (match(Arg, m_Not(m_Specific(V))) &&
- isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+ isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic)) {
assert(BitWidth == 1 && "assume operand is not i1?");
(void)BitWidth;
Known.setAllZero();
@@ -790,7 +792,7 @@ void llvm::computeKnownBitsFromContext(const Value *V, KnownBits &Known,
if (!Cmp)
continue;
- if (!isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ if (!isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic))
continue;
computeKnownBitsFromCmp(V, Cmp->getPredicate(), Cmp->getOperand(0),
@@ -4213,7 +4215,7 @@ static FPClassTest computeKnownFPClassFromAssumes(const Value *V,
assert(I->getCalledFunction()->getIntrinsicID() == Intrinsic::assume &&
"must be an assume intrinsic");
- if (!isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ if (!isValidAssumeForContext(I, Q.CxtI, Q.DT, Q.Deterministic))
continue;
CmpInst::Predicate Pred;
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index ba40ee7693636e..0eb5d364802777 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2192,8 +2192,9 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
// Treat lshr with non-negative operand as ashr.
if (match(V, m_LShr(m_Value(A), m_Value(B))) &&
- isKnownNonNegative(A, SQ.getWithInstruction(cast<Instruction>(V)),
- Depth)) {
+ isKnownNonNegative(
+ A, SQ.getWithInstruction(cast<Instruction>(V)).getWithDeterministic(),
+ Depth)) {
if (auto *AV = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateAShr(AV, B) : NonNull;
diff --git a/llvm/test/Transforms/InstCombine/pr75369.ll b/llvm/test/Transforms/InstCombine/pr75369.ll
new file mode 100644
index 00000000000000..2f90753504b36d
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr75369.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes=instcombine -S < %s | FileCheck %s
+
+define i32 @main(ptr %a, i8 %a0, i32 %conv, i8 %a1) {
+; CHECK-LABEL: define i32 @main(
+; CHECK-SAME: ptr [[A:%.*]], i8 [[A0:%.*]], i32 [[CONV:%.*]], i8 [[A1:%.*]]) {
+; CHECK-NEXT: [[A3:%.*]] = trunc i32 [[CONV]] to i8
+; CHECK-NEXT: [[OR11:%.*]] = or i8 [[A3]], [[A0]]
+; CHECK-NEXT: store i8 [[OR11]], ptr [[A]], align 1
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[A1]], 0
+; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
+; CHECK-NEXT: ret i32 [[CONV]]
+;
+ %conv1 = sext i8 %a1 to i32
+ %a2 = xor i32 %conv, 1
+ %or = or i32 %conv1, %conv
+ %not = xor i32 %or, -1
+ %shr = lshr i32 %not, 1
+ %add.neg3 = sub i32 %a2, %shr
+ %conv24 = trunc i32 %add.neg3 to i8
+ store i8 %conv24, ptr %a, align 1
+ %sext = shl i32 %conv, 0
+ %conv3 = ashr i32 %sext, 0
+ %a3 = trunc i32 %conv to i8
+ %conv5 = or i8 %a3, 0
+ %xor6 = xor i8 %conv5, 0
+ %xor816 = xor i8 %a0, 0
+ %a4 = xor i8 %xor816, 0
+ %or11 = or i8 %xor6, %a4
+ store i8 %or11, ptr %a, align 1
+ %cmp = icmp slt i8 %a1, 0
+ call void @llvm.assume(i1 %cmp)
+ ret i32 %conv3
+}
+
+declare void @llvm.assume(i1)
``````````
</details>
https://github.com/llvm/llvm-project/pull/75452
More information about the llvm-commits
mailing list