[PATCH] InstCombine: fix fold "fcmp x, undef" to account for NaN
David Majnemer
david.majnemer at gmail.com
Mon Mar 2 16:04:53 PST 2015
================
Comment at: lib/Analysis/InstructionSimplify.cpp:3044
@@ +3043,3 @@
+ // fcmp pred x, undef
+ if (isa<UndefValue>(RHS) || isa<UndefValue>(LHS)) {
+ CmpInst::Predicate Predicate = CmpInst::Predicate(Pred);
----------------
It would be nice to have `LHS` on the left and `RHS` on the right.
I think the comment would be clearer if it mentioned that this simplification will effect `fcmp pred undef, x` as well.
================
Comment at: lib/Analysis/InstructionSimplify.cpp:3045
@@ +3044,3 @@
+ if (isa<UndefValue>(RHS) || isa<UndefValue>(LHS)) {
+ CmpInst::Predicate Predicate = CmpInst::Predicate(Pred);
+ // Choosing NaN for the undef will always make unordered comparison succeed
----------------
Isn't `Pred` already a `CmpInst::Predicate`? What does this line achieve?
================
Comment at: lib/Analysis/InstructionSimplify.cpp:3047
@@ +3046,3 @@
+ // Choosing NaN for the undef will always make unordered comparison succeed
+ // and ordered comparison fails.
+ return ConstantInt::get(GetCompareTy(LHS), CmpInst::isUnordered(Predicate));
----------------
s/fails/fail/
================
Comment at: lib/IR/ConstantFold.cpp:1330
@@ -1329,3 +1329,3 @@
if (!isa<ConstantExpr>(V2)) {
- // We distilled thisUse the standard constant folder for a few cases
+ // We distilled this down to a simple case, use standard constant folder.
ConstantInt *R = nullptr;
----------------
perhaps "use the standard constant folder."
================
Comment at: lib/IR/ConstantFold.cpp:1673-1675
@@ -1673,1 +1672,5 @@
+ if (ICmpInst::isEquality(Predicate) ||
+ (ICmpInst::isIntPredicate(Predicate) && isa<UndefValue>(C1) &&
+ isa<UndefValue>(C2)))
return UndefValue::get(ResultTy);
+
----------------
Either move the `ICmpInst::isIntPredicate(Predicate) && isa<UndefValue>(C1) && isa<UndefValue>(C2)` check out of this block or simplify it with `ICmpInst::isIntPredicate(Predicate) && C1 == C2`, the line is getting a bit too long.
http://reviews.llvm.org/D7617
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list