InstCombine: fix fold "fcmp x, undef" to account for NaN

[mehdi_amini]

Summary

See the two test cases.

; Can fold fcmp with undef on one side by choosing NaN for the undef

; Can fold fcmp with undef on both side
; fcmp u_pred undef, undef -> true
; fcmp o_pred undef, undef -> false
; because whatever you choose for the first undef
; you can choose NaN for the other undef

[hfinkel]

I wonder if we could access the target machine specific info to know
if NaN are disabled?
Or what is the right way of getting the -enable-no-nans-fp-math in opt?

Check the fast-math flags on the the operand (call Op->getFastMathFlags()). The LangRef says:

LLVM IR floating-point binary ops (fadd, fsub, fmul, fdiv, frem) have the following flags that can set to enable otherwise unsafe floating point operations
...
nnan - No NaNs - Allow optimizations to assume the arguments and result are not NaN. Such optimizations are required to retain defined behavior over NaNs, but the value of the result is undefined.
...
fast - Fast - Allow algebraically equivalent transformations that may dramatically change results in floating point (e.g. reassociate). This flag implies all the others.

For constant values, you can call C->isNaN();

[mehdi_amini]

After more thought, I found out we can fold depending on if fcmp is or isn't ordered.

[mehdi_amini]

Ping.

[mehdi_amini]

Ping. Can someone look at this InstCombine change?

[chandlerc]

CC-ing David who seems like he might be in a good position to review this.

[majnemer]

lib/Analysis/InstructionSimplify.cpp

3046

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.

3047

Isn't Pred already a CmpInst::Predicate? What does this line achieve?

3049

s/fails/fail/

lib/IR/ConstantFold.cpp

1330 ↗

(On Diff #20466)

perhaps "use the standard constant folder."

1673–1675 ↗

(On Diff #20466)

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.

[mehdi_amini]

Update thanks to David's review.

[majnemer]

LGTM

lib/Analysis/InstructionSimplify.cpp

static Value *SimplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
   }
 
   // Fold trivial predicates.
   if (Pred == FCmpInst::FCMP_FALSE)
     return ConstantInt::get(GetCompareTy(LHS), 0);
   if (Pred == FCmpInst::FCMP_TRUE)
     return ConstantInt::get(GetCompareTy(LHS), 1);
 
-  if (isa<UndefValue>(RHS))                  // fcmp pred X, undef -> undef
+  // fcmp pred undef, undef -> undef
+  if (isa<UndefValue>(RHS) && isa<UndefValue>(LHS))
     return UndefValue::get(GetCompareTy(LHS));
 
   // fcmp x,x -> true/false.  Not all compares are foldable.
   if (LHS == RHS) {
     if (CmpInst::isTrueWhenEqual(Pred))
       return ConstantInt::get(GetCompareTy(LHS), 1);
     if (CmpInst::isFalseWhenEqual(Pred))
       return ConstantInt::get(GetCompareTy(LHS), 0);
   }
 
   // Handle fcmp with constant RHS
   if (ConstantFP *CFP = dyn_cast<ConstantFP>(RHS)) {
     // If the constant is a nan, see if we can fold the comparison based on it.

test/Transforms/InstCombine/fcmp.ll

 
 ; Don't crash.
 define i32 @test17(double %a, double (double)* %p) nounwind {
   %call = tail call double %p(double %a) nounwind
   %cmp = fcmp ueq double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 }
+
+; Can't fold fcmp with undef on one side
+; To be able to fold
+;   fcmp pred x, undef -> undef
+; one need to prove that for all possible value of x, there exist
+; one value for undef that makes the comparison false, and another
+; value value for undef that makes the comparison true
+; The possibility of NaN prevents the folding
+define i32 @test18_undef(float %a) nounwind {
+; CHECK-LABEL: @test18_undef
+; CHECK-NOT: ret 0
+; CHECK-NOT: ret 1
+  %cmp = fcmp ueq float %a, undef
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+}
+
+; Can fold fcmp with undef on both side
+;   fcmp pred undef, undef -> undef
+; even if one NaN is undef, you can alway chose
+define i1 @test19_undef() nounwind {
+; CHECK-LABEL: @test19_undef
+; CHECK: ret i1 undef
+  %cmp = fcmp ueq float undef, undef
+  ret i1 %cmp
+}