-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[InstSimplify] fold uno/ord comparison if fpclass is always NaN #97763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: Alex MacLean (AlexMaclean) ChangesWe already canonicalize operands that are known to be never NaN to 0.0 for Full diff: https://github.com/llvm/llvm-project/pull/97763.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index a67ad501982d2..51db54aa9b61d 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -276,6 +276,8 @@ struct KnownFPClass {
return (KnownFPClasses & Mask) == fcNone;
}
+ bool isKnownAlways(FPClassTest Mask) const { return isKnownNever(~Mask); }
+
bool isUnknown() const {
return KnownFPClasses == fcAllFlags && !SignBit;
}
@@ -285,6 +287,9 @@ struct KnownFPClass {
return isKnownNever(fcNan);
}
+ /// Return true if it's known this must always be a nan.
+ bool isKnownAlwaysNaN() const { return isKnownAlways(fcNan); }
+
/// Return true if it's known this can never be an infinity.
bool isKnownNeverInfinity() const {
return isKnownNever(fcInf);
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index abadf54a96767..040dcab931068 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -8103,13 +8103,20 @@ Instruction *InstCombinerImpl::visitFCmpInst(FCmpInst &I) {
// If we're just checking for a NaN (ORD/UNO) and have a non-NaN operand,
// then canonicalize the operand to 0.0.
if (Pred == CmpInst::FCMP_ORD || Pred == CmpInst::FCMP_UNO) {
- if (!match(Op0, m_PosZeroFP()) &&
- isKnownNeverNaN(Op0, 0, getSimplifyQuery().getWithInstruction(&I)))
- return replaceOperand(I, 0, ConstantFP::getZero(OpType));
-
- if (!match(Op1, m_PosZeroFP()) &&
- isKnownNeverNaN(Op1, 0, getSimplifyQuery().getWithInstruction(&I)))
- return replaceOperand(I, 1, ConstantFP::getZero(OpType));
+ if (!match(Op0, m_PosZeroFP())) {
+ KnownFPClass Known0 = computeKnownFPClass(Op0, fcAllFlags, &I);
+ if (Known0.isKnownNeverNaN())
+ return replaceOperand(I, 0, ConstantFP::getZero(OpType));
+ if (Known0.isKnownAlwaysNaN())
+ return replaceOperand(I, 0, ConstantFP::getNaN(OpType));
+ }
+ if (!match(Op1, m_PosZeroFP())) {
+ KnownFPClass Known1 = computeKnownFPClass(Op1, fcAllFlags, &I);
+ if (Known1.isKnownNeverNaN())
+ return replaceOperand(I, 1, ConstantFP::getZero(OpType));
+ if (Known1.isKnownAlwaysNaN())
+ return replaceOperand(I, 1, ConstantFP::getNaN(OpType));
+ }
}
// fcmp pred (fneg X), (fneg Y) -> fcmp swap(pred) X, Y
diff --git a/llvm/test/Transforms/InstCombine/fcmp-special.ll b/llvm/test/Transforms/InstCombine/fcmp-special.ll
index 64bc86f4266c7..6c196534931f5 100644
--- a/llvm/test/Transforms/InstCombine/fcmp-special.ll
+++ b/llvm/test/Transforms/InstCombine/fcmp-special.ll
@@ -186,6 +186,22 @@ define i1 @nnan_ops_to_fcmp_uno(float %x, float %y) {
ret i1 %cmp
}
+define i1 @nan_ops_to_fcmp_ord(float nofpclass(inf zero sub norm) %x, float nofpclass(inf zero sub norm) %y) {
+; CHECK-LABEL: @nan_ops_to_fcmp_ord(
+; CHECK-NEXT: ret i1 false
+;
+ %cmp = fcmp ord float %x, %y
+ ret i1 %cmp
+}
+
+define i1 @nan_ops_to_fcmp_uno(float nofpclass(inf zero sub norm) %x, float nofpclass(inf zero sub norm) %y) {
+; CHECK-LABEL: @nan_ops_to_fcmp_uno(
+; CHECK-NEXT: ret i1 true
+;
+ %cmp = fcmp uno float %x, %y
+ ret i1 %cmp
+}
+
; TODO: For any predicate/type/FMF, comparison to -0.0 is the same as comparison to +0.0.
define i1 @negative_zero_oeq(float %x) {
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please attach alive2 proofs and pre-commit tests.
Okay, I've attached a proof, could you explain further about what you mean by pre-commit tests? |
See https://llvm.org/docs/InstCombineContributorGuide.html#precommit-tests. |
e7a5762
to
add8cee
Compare
Thanks, I've split the tests out into a separate commit as described, please let me know if I've made any mistake. |
Please update the title and description, since this PR folds the pattern into a constant instead of canonicalizing its operands. |
add8cee
to
733fa90
Compare
733fa90
to
b54f92f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please wait for additional approval from other reviewers :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…#97763) In InstSimplify we already fold `fcmp ord/uno` to a constant when both operands are known to be non-NaN. This change slightly generalizes this to also handle the case where either of the operands is known to always be NaN. Proof: https://alive2.llvm.org/ce/z/AhCmJN
In InstSimplify we already fold
fcmp ord/uno
to a constant when both operands are known to be non-NaN. This change slightly generalizes this to also handle the case where either of the operands is known to always be NaN.Proof: https://alive2.llvm.org/ce/z/AhCmJN