[ValueTracking] Handle and/or of conditions in computeKnownFPClassFromContext
#118257
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
Your org has enabled the Graphite merge queue for merging into mainAdd the label “FP Bundles” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
|
@llvm/pr-subscribers-llvm-transforms Author: Yingwei Zheng (dtcxzyw) ChangesFix a typo introduced by #83161. Full diff: https://github.com/llvm/llvm-project/pull/118257.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c48068afc04816..f74109f4989cbb 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4885,6 +4885,13 @@ static void computeKnownFPClassFromCond(const Value *V, Value *Cond,
bool CondIsTrue,
const Instruction *CxtI,
KnownFPClass &KnownFromContext) {
+ Value *A, *B;
+ if (CondIsTrue ? match(Cond, m_LogicalAnd(m_Value(A), m_Value(B)))
+ : match(Cond, m_LogicalOr(m_Value(A), m_Value(B)))) {
+ computeKnownFPClassFromCond(V, A, CondIsTrue, CxtI, KnownFromContext);
+ computeKnownFPClassFromCond(V, B, CondIsTrue, CxtI, KnownFromContext);
+ return;
+ }
CmpInst::Predicate Pred;
Value *LHS;
uint64_t ClassVal = 0;
@@ -10090,7 +10097,7 @@ void llvm::findValuesAffectedByCondition(
if (HasRHSC && match(A, m_Intrinsic<Intrinsic::ctpop>(m_Value(X))))
AddAffected(X);
- } else if (match(Cond, m_FCmp(Pred, m_Value(A), m_Value(B)))) {
+ } else if (match(V, m_FCmp(Pred, m_Value(A), m_Value(B)))) {
AddCmpOperands(A, B);
// fcmp fneg(x), y
diff --git a/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll b/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll
index d6706d76056eea..141b44cbbb7a1f 100644
--- a/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll
+++ b/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll
@@ -46,6 +46,31 @@ if.end:
ret i1 %cmp.i
}
+define i1 @test2_or(double %x, i1 %cond) {
+; CHECK-LABEL: define i1 @test2_or(
+; CHECK-SAME: double [[X:%.*]], i1 [[COND:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = fcmp olt double [[X]], 0x3EB0C6F7A0000000
+; CHECK-NEXT: [[OR:%.*]] = or i1 [[CMP]], [[COND]]
+; CHECK-NEXT: br i1 [[OR]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: ret i1 false
+; CHECK: if.end:
+; CHECK-NEXT: ret i1 false
+;
+entry:
+ %cmp = fcmp olt double %x, 0x3EB0C6F7A0000000
+ %or = or i1 %cmp, %cond
+ br i1 %or, label %if.then, label %if.end
+
+if.then:
+ ret i1 false
+
+if.end:
+ %cmp.i = fcmp oeq double %x, 0.000000e+00
+ ret i1 %cmp.i
+}
+
define i1 @test3(float %x) {
; CHECK-LABEL: define i1 @test3(
; CHECK-SAME: float [[X:%.*]]) {
@@ -240,7 +265,6 @@ if.else:
ret i1 false
}
-; TODO: handle and/or conditions
define i1 @test11_and(float %x, i1 %cond2) {
; CHECK-LABEL: define i1 @test11_and(
; CHECK-SAME: float [[X:%.*]], i1 [[COND2:%.*]]) {
@@ -248,8 +272,7 @@ define i1 @test11_and(float %x, i1 %cond2) {
; CHECK-NEXT: [[AND:%.*]] = and i1 [[COND]], [[COND2]]
; CHECK-NEXT: br i1 [[AND]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: [[RET1:%.*]] = fcmp oeq float [[X]], 0x7FF0000000000000
-; CHECK-NEXT: ret i1 [[RET1]]
+; CHECK-NEXT: ret i1 false
; CHECK: if.else:
; CHECK-NEXT: ret i1 false
;
@@ -264,7 +287,6 @@ if.else:
ret i1 false
}
-; TODO: handle and/or conditions
define i1 @test12_or(float %x, i1 %cond2) {
; CHECK-LABEL: define i1 @test12_or(
; CHECK-SAME: float [[X:%.*]], i1 [[COND2:%.*]]) {
@@ -275,7 +297,7 @@ define i1 @test12_or(float %x, i1 %cond2) {
; CHECK: if.then:
; CHECK-NEXT: ret i1 false
; CHECK: if.else:
-; CHECK-NEXT: [[RET:%.*]] = call i1 @llvm.is.fpclass.f32(float [[X]], i32 783)
+; CHECK-NEXT: [[RET:%.*]] = call i1 @llvm.is.fpclass.f32(float [[X]], i32 780)
; CHECK-NEXT: ret i1 [[RET]]
;
entry:
|
|
@llvm/pr-subscribers-llvm-analysis Author: Yingwei Zheng (dtcxzyw) ChangesFix a typo introduced by #83161. Full diff: https://github.com/llvm/llvm-project/pull/118257.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c48068afc04816..f74109f4989cbb 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4885,6 +4885,13 @@ static void computeKnownFPClassFromCond(const Value *V, Value *Cond,
bool CondIsTrue,
const Instruction *CxtI,
KnownFPClass &KnownFromContext) {
+ Value *A, *B;
+ if (CondIsTrue ? match(Cond, m_LogicalAnd(m_Value(A), m_Value(B)))
+ : match(Cond, m_LogicalOr(m_Value(A), m_Value(B)))) {
+ computeKnownFPClassFromCond(V, A, CondIsTrue, CxtI, KnownFromContext);
+ computeKnownFPClassFromCond(V, B, CondIsTrue, CxtI, KnownFromContext);
+ return;
+ }
CmpInst::Predicate Pred;
Value *LHS;
uint64_t ClassVal = 0;
@@ -10090,7 +10097,7 @@ void llvm::findValuesAffectedByCondition(
if (HasRHSC && match(A, m_Intrinsic<Intrinsic::ctpop>(m_Value(X))))
AddAffected(X);
- } else if (match(Cond, m_FCmp(Pred, m_Value(A), m_Value(B)))) {
+ } else if (match(V, m_FCmp(Pred, m_Value(A), m_Value(B)))) {
AddCmpOperands(A, B);
// fcmp fneg(x), y
diff --git a/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll b/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll
index d6706d76056eea..141b44cbbb7a1f 100644
--- a/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll
+++ b/llvm/test/Transforms/InstCombine/fpclass-from-dom-cond.ll
@@ -46,6 +46,31 @@ if.end:
ret i1 %cmp.i
}
+define i1 @test2_or(double %x, i1 %cond) {
+; CHECK-LABEL: define i1 @test2_or(
+; CHECK-SAME: double [[X:%.*]], i1 [[COND:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = fcmp olt double [[X]], 0x3EB0C6F7A0000000
+; CHECK-NEXT: [[OR:%.*]] = or i1 [[CMP]], [[COND]]
+; CHECK-NEXT: br i1 [[OR]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: ret i1 false
+; CHECK: if.end:
+; CHECK-NEXT: ret i1 false
+;
+entry:
+ %cmp = fcmp olt double %x, 0x3EB0C6F7A0000000
+ %or = or i1 %cmp, %cond
+ br i1 %or, label %if.then, label %if.end
+
+if.then:
+ ret i1 false
+
+if.end:
+ %cmp.i = fcmp oeq double %x, 0.000000e+00
+ ret i1 %cmp.i
+}
+
define i1 @test3(float %x) {
; CHECK-LABEL: define i1 @test3(
; CHECK-SAME: float [[X:%.*]]) {
@@ -240,7 +265,6 @@ if.else:
ret i1 false
}
-; TODO: handle and/or conditions
define i1 @test11_and(float %x, i1 %cond2) {
; CHECK-LABEL: define i1 @test11_and(
; CHECK-SAME: float [[X:%.*]], i1 [[COND2:%.*]]) {
@@ -248,8 +272,7 @@ define i1 @test11_and(float %x, i1 %cond2) {
; CHECK-NEXT: [[AND:%.*]] = and i1 [[COND]], [[COND2]]
; CHECK-NEXT: br i1 [[AND]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: [[RET1:%.*]] = fcmp oeq float [[X]], 0x7FF0000000000000
-; CHECK-NEXT: ret i1 [[RET1]]
+; CHECK-NEXT: ret i1 false
; CHECK: if.else:
; CHECK-NEXT: ret i1 false
;
@@ -264,7 +287,6 @@ if.else:
ret i1 false
}
-; TODO: handle and/or conditions
define i1 @test12_or(float %x, i1 %cond2) {
; CHECK-LABEL: define i1 @test12_or(
; CHECK-SAME: float [[X:%.*]], i1 [[COND2:%.*]]) {
@@ -275,7 +297,7 @@ define i1 @test12_or(float %x, i1 %cond2) {
; CHECK: if.then:
; CHECK-NEXT: ret i1 false
; CHECK: if.else:
-; CHECK-NEXT: [[RET:%.*]] = call i1 @llvm.is.fpclass.f32(float [[X]], i32 783)
+; CHECK-NEXT: [[RET:%.*]] = call i1 @llvm.is.fpclass.f32(float [[X]], i32 780)
; CHECK-NEXT: ret i1 [[RET]]
;
entry:
|
nikic
reviewed
Dec 2, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/1779 Here is the relevant piece of the build log for the reference |
|
Heads-up: this function changes behavior when optimized after this commit. Precisely when optimized + using msan on k8. |
|
@dtcxzyw please find the attached repro https://godbolt.org/z/oPdnz87va |
Sorry I missed this comment. I will have a look. |
|
Reduced reproducer: https://godbolt.org/z/7Md6ov4oG |
|
Alive2: https://alive2.llvm.org/ce/z/rPDdR_ |
dtcxzyw
added a commit
that referenced
this pull request
Dec 12, 2024
…mCond` (#119579) After #118257, we may call `computeKnownFPClassFromCond` with unrelated conditions. Then miscompilations may occur due to a lack of operand checks. This bug was introduced by d2404ea and #80740. However, the miscompilation couldn't have happened before #118257, because we only added related conditions to `DomConditionCache/AssumptionCache`. Fix the miscompilation reported in #118257 (comment).
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.
Fix a typo introduced by #83161.
This patch also supports decomposition of and/or expressions in
computeKnownFPClassFromContext.Compile-time improvement: http://llvm-compile-time-tracker.com/compare.php?from=688bb432c4b618de69a1d0e7807077a22f15762a&to=07493fc354b686f0aca79d6f817091a757bd7cd5&stat=instructions:u