[llvm] 312a532 - [GVN/FP] Considate logic for reasoning about equality vs equivalance for floats
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 7 16:05:10 PST 2020
Author: Philip Reames
Date: 2020-01-07T16:05:04-08:00
New Revision: 312a532dc0456b8901de43fd3f1c6ec9d551a80d
URL: https://github.com/llvm/llvm-project/commit/312a532dc0456b8901de43fd3f1c6ec9d551a80d
DIFF: https://github.com/llvm/llvm-project/commit/312a532dc0456b8901de43fd3f1c6ec9d551a80d.diff
LOG: [GVN/FP] Considate logic for reasoning about equality vs equivalance for floats
Factor out common logic into some reasonable commented helper functions. In the process, ensure that the in-block vs cross-block cases are handled the same. They previously weren't.
Differential Revision: https://reviews.llvm.org/D67126
Added:
Modified:
llvm/lib/Transforms/Scalar/GVN.cpp
llvm/test/Transforms/GVN/edge.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 46947fe5c94c..1e6aab14e7b4 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1389,6 +1389,59 @@ bool GVN::processNonLocalLoad(LoadInst *LI) {
return PerformLoadPRE(LI, ValuesPerBlock, UnavailableBlocks);
}
+static bool impliesEquivalanceIfTrue(CmpInst* Cmp) {
+ if (Cmp->getPredicate() == CmpInst::Predicate::ICMP_EQ)
+ return true;
+
+ // Floating point comparisons can be equal, but not equivalent. Cases:
+ // NaNs for unordered operators
+ // +0.0 vs 0.0 for all operators
+ if (Cmp->getPredicate() == CmpInst::Predicate::FCMP_OEQ ||
+ (Cmp->getPredicate() == CmpInst::Predicate::FCMP_UEQ &&
+ Cmp->getFastMathFlags().noNaNs())) {
+ Value *LHS = Cmp->getOperand(0);
+ Value *RHS = Cmp->getOperand(1);
+ // If we can prove either side non-zero, then equality must imply
+ // equivalence.
+ // FIXME: We should do this optimization if 'no signed zeros' is
+ // applicable via an instruction-level fast-math-flag or some other
+ // indicator that relaxed FP semantics are being used.
+ if (isa<ConstantFP>(LHS) && !cast<ConstantFP>(LHS)->isZero())
+ return true;
+ if (isa<ConstantFP>(RHS) && !cast<ConstantFP>(RHS)->isZero())
+ return true;;
+ // TODO: Handle vector floating point constants
+ }
+ return false;
+}
+
+static bool impliesEquivalanceIfFalse(CmpInst* Cmp) {
+ if (Cmp->getPredicate() == CmpInst::Predicate::ICMP_NE)
+ return true;
+
+ // Floating point comparisons can be equal, but not equivelent. Cases:
+ // NaNs for unordered operators
+ // +0.0 vs 0.0 for all operators
+ if ((Cmp->getPredicate() == CmpInst::Predicate::FCMP_ONE &&
+ Cmp->getFastMathFlags().noNaNs()) ||
+ Cmp->getPredicate() == CmpInst::Predicate::FCMP_UNE) {
+ Value *LHS = Cmp->getOperand(0);
+ Value *RHS = Cmp->getOperand(1);
+ // If we can prove either side non-zero, then equality must imply
+ // equivalence.
+ // FIXME: We should do this optimization if 'no signed zeros' is
+ // applicable via an instruction-level fast-math-flag or some other
+ // indicator that relaxed FP semantics are being used.
+ if (isa<ConstantFP>(LHS) && !cast<ConstantFP>(LHS)->isZero())
+ return true;
+ if (isa<ConstantFP>(RHS) && !cast<ConstantFP>(RHS)->isZero())
+ return true;;
+ // TODO: Handle vector floating point constants
+ }
+ return false;
+}
+
+
static bool hasUsersIn(Value *V, BasicBlock *BB) {
for (User *U : V->users())
if (isa<Instruction>(U) &&
@@ -1453,10 +1506,7 @@ bool GVN::processAssumeIntrinsic(IntrinsicInst *IntrinsicI) {
// call void @llvm.assume(i1 %cmp)
// ret float %load ; will change it to ret float %0
if (auto *CmpI = dyn_cast<CmpInst>(V)) {
- if (CmpI->getPredicate() == CmpInst::Predicate::ICMP_EQ ||
- CmpI->getPredicate() == CmpInst::Predicate::FCMP_OEQ ||
- (CmpI->getPredicate() == CmpInst::Predicate::FCMP_UEQ &&
- CmpI->getFastMathFlags().noNaNs())) {
+ if (impliesEquivalanceIfTrue(CmpI)) {
Value *CmpLHS = CmpI->getOperand(0);
Value *CmpRHS = CmpI->getOperand(1);
// Heuristically pick the better replacement -- the choice of heuristic
@@ -1483,12 +1533,6 @@ bool GVN::processAssumeIntrinsic(IntrinsicInst *IntrinsicI) {
if (isa<Constant>(CmpLHS) && isa<Constant>(CmpRHS))
return Changed;
- // +0.0 and -0.0 compare equal, but do not imply equivalence. Unless we
- // can prove equivalence, bail.
- if (CmpRHS->getType()->isFloatTy() &&
- (!isa<ConstantFP>(CmpRHS) || cast<ConstantFP>(CmpRHS)->isZero()))
- return Changed;
-
LLVM_DEBUG(dbgs() << "Replacing dominated uses of "
<< *CmpLHS << " with "
<< *CmpRHS << " in block "
@@ -1877,27 +1921,12 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root,
Value *Op0 = Cmp->getOperand(0), *Op1 = Cmp->getOperand(1);
// If "A == B" is known true, or "A != B" is known false, then replace
- // A with B everywhere in the scope.
- if ((isKnownTrue && Cmp->getPredicate() == CmpInst::ICMP_EQ) ||
- (isKnownFalse && Cmp->getPredicate() == CmpInst::ICMP_NE))
+ // A with B everywhere in the scope. For floating point operations, we
+ // have to be careful since equality does not always imply equivalance.
+ if ((isKnownTrue && impliesEquivalanceIfTrue(Cmp)) ||
+ (isKnownFalse && impliesEquivalanceIfFalse(Cmp)))
Worklist.push_back(std::make_pair(Op0, Op1));
- // Handle the floating point versions of equality comparisons too.
- if ((isKnownTrue && Cmp->getPredicate() == CmpInst::FCMP_OEQ) ||
- (isKnownFalse && Cmp->getPredicate() == CmpInst::FCMP_UNE)) {
-
- // Floating point -0.0 and 0.0 compare equal, so we can only
- // propagate values if we know that we have a constant and that
- // its value is non-zero.
-
- // FIXME: We should do this optimization if 'no signed zeros' is
- // applicable via an instruction-level fast-math-flag or some other
- // indicator that relaxed FP semantics are being used.
-
- if (isa<ConstantFP>(Op1) && !cast<ConstantFP>(Op1)->isZero())
- Worklist.push_back(std::make_pair(Op0, Op1));
- }
-
// If "A >= B" is known true, replace "A < B" with false everywhere.
CmpInst::Predicate NotPred = Cmp->getInversePredicate();
Constant *NotVal = ConstantInt::get(Cmp->getType(), isKnownFalse);
diff --git a/llvm/test/Transforms/GVN/edge.ll b/llvm/test/Transforms/GVN/edge.ll
index 0c1a3fbec528..1d9407b788dc 100644
--- a/llvm/test/Transforms/GVN/edge.ll
+++ b/llvm/test/Transforms/GVN/edge.ll
@@ -93,6 +93,40 @@ return:
; CHECK: %div = fdiv double %x, 2.0
}
+define double @fcmp_one_possibly_nan(double %x, double %y) {
+entry:
+ %cmp = fcmp one double %y, 2.0
+ br i1 %cmp, label %return, label %else
+
+else:
+ %div = fdiv double %x, %y
+ br label %return
+
+return:
+ %retval = phi double [ %div, %else ], [ %x, %entry ]
+ ret double %retval
+
+; CHECK-LABEL: define double @fcmp_one_possibly_nan(
+; CHECK: %div = fdiv double %x, %y
+}
+
+define double @fcmp_one_not_zero_or_nan(double %x, double %y) {
+entry:
+ %cmp = fcmp nnan one double %y, 2.0
+ br i1 %cmp, label %return, label %else
+
+else:
+ %div = fdiv double %x, %y
+ br label %return
+
+return:
+ %retval = phi double [ %div, %else ], [ %x, %entry ]
+ ret double %retval
+
+; CHECK-LABEL: define double @fcmp_one_not_zero_or_nan(
+; CHECK: %div = fdiv double %x, 2.0
+}
+
; PR22376 - We can't propagate zero constants because -0.0
; compares equal to 0.0. If %y is -0.0 in this test case,
; we would produce the wrong sign on the infinity return value.
@@ -168,3 +202,38 @@ return:
; CHECK-LABEL: define double @fcmp_une_maybe_zero(
; CHECK: %div = fdiv double %x, %z
}
+
+
+define double @fcmp_ueq_possibly_nan(double %x, double %y) {
+entry:
+ %cmp = fcmp ueq double %y, 2.0
+ br i1 %cmp, label %do_div, label %return
+
+do_div:
+ %div = fdiv double %x, %y
+ br label %return
+
+return:
+ %retval = phi double [ %div, %do_div ], [ %x, %entry ]
+ ret double %retval
+
+; CHECK-LABEL: define double @fcmp_ueq_possibly_nan(
+; CHECK: %div = fdiv double %x, %y
+}
+
+define double @fcmp_ueq_not_zero_or_nan(double %x, double %y) {
+entry:
+ %cmp = fcmp nnan ueq double %y, 2.0
+ br i1 %cmp, label %do_div, label %return
+
+do_div:
+ %div = fdiv double %x, %y
+ br label %return
+
+return:
+ %retval = phi double [ %div, %do_div ], [ %x, %entry ]
+ ret double %retval
+
+; CHECK-LABEL: define double @fcmp_ueq_not_zero_or_nan(
+; CHECK: %div = fdiv double %x, 2.0
+}
More information about the llvm-commits
mailing list