[ValueTracking] Refactor isKnownNonEqualFromContext
#127388
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
llvm:analysis
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Yingwei Zheng (dtcxzyw) ChangesThis patch avoids adding RHS for comparisons with two variable operands (#118493 (comment)). Instead, we iterate over related dominating conditions of both V1 and V2 in Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u Full diff: https://github.com/llvm/llvm-project/pull/127388.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 2a49a10447e0b..c1d3e9fd7e010 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -3786,6 +3786,63 @@ static bool isNonEqualPointersWithRecursiveGEP(const Value *A, const Value *B,
(StartOffset.sle(OffsetB) && StepOffset.isNegative()));
}
+static bool isKnownNonEqualFromContext(const Value *V1, const Value *V2,
+ unsigned Depth, const SimplifyQuery &Q) {
+ if (!Q.CxtI)
+ return false;
+
+ // Try to infer NonEqual based on information from dominating conditions.
+ if (Q.DC && Q.DT) {
+ auto IsKnownNonEqualFromDominatingCondition = [&](const Value *V) {
+ for (BranchInst *BI : Q.DC->conditionsFor(V)) {
+ Value *Cond = BI->getCondition();
+ BasicBlockEdge Edge0(BI->getParent(), BI->getSuccessor(0));
+ if (Q.DT->dominates(Edge0, Q.CxtI->getParent()) &&
+ isImpliedCondition(Cond, ICmpInst::ICMP_NE, V1, V2, Q.DL,
+ /*LHSIsTrue=*/true, Depth)
+ .value_or(false))
+ return true;
+
+ BasicBlockEdge Edge1(BI->getParent(), BI->getSuccessor(1));
+ if (Q.DT->dominates(Edge1, Q.CxtI->getParent()) &&
+ isImpliedCondition(Cond, ICmpInst::ICMP_NE, V1, V2, Q.DL,
+ /*LHSIsTrue=*/false, Depth)
+ .value_or(false))
+ return true;
+ }
+
+ return false;
+ };
+
+ if (IsKnownNonEqualFromDominatingCondition(V1) ||
+ IsKnownNonEqualFromDominatingCondition(V2))
+ return true;
+ }
+
+ if (!Q.AC)
+ return false;
+
+ // Try to infer NonEqual based on information from assumptions.
+ for (auto &AssumeVH : Q.AC->assumptionsFor(V1)) {
+ if (!AssumeVH)
+ continue;
+ CallInst *I = cast<CallInst>(AssumeVH);
+
+ assert(I->getFunction() == Q.CxtI->getFunction() &&
+ "Got assumption for the wrong function!");
+ assert(I->getIntrinsicID() == Intrinsic::assume &&
+ "must be an assume intrinsic");
+
+ if (isImpliedCondition(I->getArgOperand(0), ICmpInst::ICMP_NE, V1, V2, Q.DL,
+ /*LHSIsTrue=*/true, Depth)
+ .value_or(false) &&
+ isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ return true;
+ }
+
+ return false;
+}
+
/// Return true if it is known that V1 != V2.
static bool isKnownNonEqual(const Value *V1, const Value *V2,
const APInt &DemandedElts, unsigned Depth,
@@ -3857,49 +3914,8 @@ static bool isKnownNonEqual(const Value *V1, const Value *V2,
match(V2, m_PtrToIntSameSize(Q.DL, m_Value(B))))
return isKnownNonEqual(A, B, DemandedElts, Depth + 1, Q);
- if (!Q.CxtI)
- return false;
-
- // Try to infer NonEqual based on information from dominating conditions.
- if (Q.DC && Q.DT) {
- for (BranchInst *BI : Q.DC->conditionsFor(V1)) {
- Value *Cond = BI->getCondition();
- BasicBlockEdge Edge0(BI->getParent(), BI->getSuccessor(0));
- if (Q.DT->dominates(Edge0, Q.CxtI->getParent()) &&
- isImpliedCondition(Cond, ICmpInst::ICMP_NE, V1, V2, Q.DL,
- /*LHSIsTrue=*/true, Depth)
- .value_or(false))
- return true;
-
- BasicBlockEdge Edge1(BI->getParent(), BI->getSuccessor(1));
- if (Q.DT->dominates(Edge1, Q.CxtI->getParent()) &&
- isImpliedCondition(Cond, ICmpInst::ICMP_NE, V1, V2, Q.DL,
- /*LHSIsTrue=*/false, Depth)
- .value_or(false))
- return true;
- }
- }
-
- if (!Q.AC)
- return false;
-
- // Try to infer NonEqual based on information from assumptions.
- for (auto &AssumeVH : Q.AC->assumptionsFor(V1)) {
- if (!AssumeVH)
- continue;
- CallInst *I = cast<CallInst>(AssumeVH);
-
- assert(I->getFunction() == Q.CxtI->getFunction() &&
- "Got assumption for the wrong function!");
- assert(I->getIntrinsicID() == Intrinsic::assume &&
- "must be an assume intrinsic");
-
- if (isImpliedCondition(I->getArgOperand(0), ICmpInst::ICMP_NE, V1, V2, Q.DL,
- /*LHSIsTrue=*/true, Depth)
- .value_or(false) &&
- isValidAssumeForContext(I, Q.CxtI, Q.DT))
- return true;
- }
+ if (isKnownNonEqualFromContext(V1, V2, Depth, Q))
+ return true;
return false;
}
@@ -10278,7 +10294,8 @@ void llvm::findValuesAffectedByCondition(
bool HasRHSC = match(B, m_ConstantInt());
if (ICmpInst::isEquality(Pred)) {
AddAffected(A);
- AddAffected(B);
+ if (IsAssume)
+ AddAffected(B);
if (HasRHSC) {
Value *Y;
// (X & C) or (X | C).
|
artagnon
reviewed
Feb 16, 2025
nikic
reviewed
Feb 16, 2025
It is not NFC. See dtcxzyw/llvm-opt-benchmark#2121. The net effect is positive. |
Could add some tests extracted from the diffs? |
|
Case 1 (regression): With this patch, we do not add |
|
Case 2 (improvement): With this patch, we iterate over related dominating conditions of |
llvmbot
added
llvm:instcombine
nikic
reviewed
Mar 1, 2025
There was a problem hiding this comment.
Okay, so the differences here are because isImpliedCondition can also use things other than equality conditions, and we may pick up different "unrelated" conditions depending on how we query things.
I don't really have a strong opinion on how this query is done, so this approach is fine by me.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/14720 Here is the relevant piece of the build log for the reference |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This patch avoids adding RHS for comparisons with two variable operands (llvm#118493 (comment)). Instead, we iterate over related dominating conditions of both V1 and V2 in `isKnownNonEqualFromContext`, as suggested by goldsteinn (llvm#117442 (comment)). Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This patch avoids adding RHS for comparisons with two variable operands (llvm#118493 (comment)). Instead, we iterate over related dominating conditions of both V1 and V2 in `isKnownNonEqualFromContext`, as suggested by goldsteinn (llvm#117442 (comment)). Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This patch avoids adding RHS for comparisons with two variable operands (llvm#118493 (comment)). Instead, we iterate over related dominating conditions of both V1 and V2 in `isKnownNonEqualFromContext`, as suggested by goldsteinn (llvm#117442 (comment)). Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch avoids adding RHS for comparisons with two variable operands (#118493 (comment)). Instead, we iterate over related dominating conditions of both V1 and V2 in
isKnownNonEqualFromContext, as suggested by goldsteinn (#117442 (comment)).Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u