Broaden optimization of fcmp ([us]itofp x, constant)

[ArchDRobison]

Summary

The existing instruction-combining optimization FoldFCmp_IntToFP_Cst simplifies fcmp ([us]itofp x, constant) punts if the conversion of x might lose accuracy, unless the constant is positive zero. This patch makes the "lose accuracy" test slightly less conservative and lifts restrictions on the constant so that the following additional cases are handled:

• the constant's magnitude is so small that rounding of x is irrelevant.
• the constant's magnitude is so large that the rounding of x is irrelevant.

The first case includes negative zero. The last case includes comparing against infinity. Note that sufficiently large integers can exceed a floating-point format's range, so the last case still requires care. See the new i128 test for an example.

The patch just inspects the exponent of the constant, and so still leaves some optimizable cases behind, but those seem not worth adding further complexity.

Motivation for patch is a Julia issue https://github.com/JuliaLang/julia/issues/11987 . See thread starting with http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-July/087630.html for more LLVM discussion.

Who is the code owner for this optimization?

[ArchDRobison]

Ping. Just a reminder that this patch needs review and approval. I've had one floating-point expert, but who does not work on LLVM, look it over so far ( https://github.com/JuliaLang/julia/issues/11987#issuecomment-124238779 ).

[ArchDRobison]

Updated as diff against 2015-8-24 trunk. No changes to logic.

[majnemer]

Adding @chandlerc and @scanon.

Could either of you (preferably both of you) take a look at this?

[scanon]

Looks good to me. Apologies for delay in getting to this one.

[Eugene.Zelenko]

Committed in rL247708.

lib/Transforms/InstCombine/InstCombineCompares.cpp

Instruction *InstCombiner::FoldFCmp_IntToFP_Cst(FCmpInst &I,
 
   // Get the width of the mantissa.  We don't want to hack on conversions that
   // might lose information from the integer, e.g. "i64 -> float"
   int MantissaWidth = LHSI->getType()->getFPMantissaWidth();
   if (MantissaWidth == -1) return nullptr;  // Unknown.
 
   IntegerType *IntTy = cast<IntegerType>(LHSI->getOperand(0)->getType());
 
-  // Check to see that the input is converted from an integer type that is small
-  // enough that preserves all bits.  TODO: check here for "known" sign bits.
-  // This would allow us to handle (fptosi (x >>s 62) to float) if x is i64 f.e.
-  unsigned InputSize = IntTy->getScalarSizeInBits();
-
-  // If this is a uitofp instruction, we need an extra bit to hold the sign.
   bool LHSUnsigned = isa<UIToFPInst>(LHSI);
-  if (LHSUnsigned)
-    ++InputSize;
 
   if (I.isEquality()) {
     FCmpInst::Predicate P = I.getPredicate();
     bool IsExact = false;
     APSInt RHSCvt(IntTy->getBitWidth(), LHSUnsigned);
     RHS.convertToInteger(RHSCvt, APFloat::rmNearestTiesToEven, &IsExact);
 
     // If the floating point constant isn't an integer value, we know if we will
if (!IsExact) {
         return ReplaceInstUsesWith(I, Builder->getTrue());
       }
     }
 
     // TODO: If the constant is exactly representable, is it always OK to do
     // equality compares as integer?
   }
 
-  // Comparisons with zero are a special case where we know we won't lose
-  // information.
-  bool IsCmpZero = RHS.isPosZero();
+  // Check to see that the input is converted from an integer type that is small
+  // enough that preserves all bits.  TODO: check here for "known" sign bits.
+  // This would allow us to handle (fptosi (x >>s 62) to float) if x is i64 f.e.
+  unsigned InputSize = IntTy->getScalarSizeInBits();
 
-  // If the conversion would lose info, don't hack on this.
-  if ((int)InputSize > MantissaWidth && !IsCmpZero)
+  // Following test does NOT adjust InputSize downwards for signed inputs, 
+  // because the most negative value still requires all the mantissa bits 
+  // to distinguish it from one less than that value.
+  if ((int)InputSize > MantissaWidth) {
+    // Conversion would lose accuracy. Check if loss can impact comparison.
+    int Exp = ilogb(RHS);
+    if (Exp == APFloat::IEK_Inf) {
+      int MaxExponent = ilogb(APFloat::getLargest(RHS.getSemantics()));
+      if (MaxExponent < (int)InputSize - !LHSUnsigned) 
+        // Conversion could create infinity.
+        return nullptr;
+    } else {
+      // Note that if RHS is zero or NaN, then Exp is negative 
+      // and first condition is trivially false.
+      if (MantissaWidth <= Exp && Exp <= (int)InputSize - !LHSUnsigned) 
+        // Conversion could affect comparison.
         return nullptr;
+    }
+  }
 
   // Otherwise, we can potentially simplify the comparison.  We know that it
   // will always come through as an integer value and we know the constant is
   // not a NAN (it would have been previously simplified).
   assert(!RHS.isNaN() && "NaN comparison not already folded!");
 
   ICmpInst::Predicate Pred;
   switch (I.getPredicate()) {

test/Transforms/InstCombine/cast-int-fcmp-eq-0.ll

 ; RUN: opt -S -instcombine < %s | FileCheck %s
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_0_uitofp(
 ; CHECK-NEXT: icmp eq i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_n0_uitofp(
-; CHECK: uitofp
-; CHECK: fcmp oeq
+; CHECK-NEXT: icmp eq i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_n0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp oeq float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_0_sitofp(
 ; CHECK-NEXT: icmp eq i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_n0_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp oeq
+; CHECK-NEXT: icmp eq i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_n0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp oeq float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_one_int_0_uitofp(
 ; CHECK-NEXT: icmp ne i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_one_int_0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp one float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_one_int_n0_uitofp(
-; CHECK: uitofp
-; CHECK: fcmp one
+; CHECK-NEXT: icmp ne i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_one_int_n0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp one float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_one_int_0_sitofp(
 ; CHECK-NEXT: icmp ne i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_one_int_0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp one float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_one_int_n0_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp one
+; CHECK-NEXT: icmp ne i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_one_int_n0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp one float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ueq_int_0_uitofp(
 ; CHECK-NEXT: icmp eq i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ueq_int_0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp ueq float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ueq_int_n0_uitofp(
-; CHECK: uitofp
-; CHECK: fcmp ueq
+; CHECK-NEXT: icmp eq i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ueq_int_n0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp ueq float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ueq_int_0_sitofp(
 ; CHECK-NEXT: icmp eq i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ueq_int_0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp ueq float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ueq_int_n0_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp ueq
+; CHECK-NEXT: icmp eq i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ueq_int_n0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp ueq float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_une_int_0_uitofp(
 ; CHECK-NEXT: icmp ne i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_une_int_0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp une float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_une_int_n0_uitofp(
-; CHECK: uitofp
-; CHECK: fcmp une
+; CHECK-NEXT: icmp ne i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_une_int_n0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp une float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_une_int_0_sitofp(
 ; CHECK-NEXT: icmp ne i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_une_int_0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp une float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_une_int_n0_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp une
+; CHECK-NEXT: icmp ne i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_une_int_n0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp une float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ogt_int_0_uitofp(
 ; CHECK: icmp ne i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ogt_int_0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp ogt float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ogt_int_n0_uitofp(
-; CHECK: uitofp
-; CHECK: fcmp ogt
+; CHECK: icmp ne i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ogt_int_n0_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp ogt float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ogt_int_0_sitofp(
 ; CHECK: icmp sgt i32 %i, 0
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ogt_int_0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp ogt float %f, 0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ogt_int_n0_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp ogt
+; CHECK: icmp sgt i32 %i, 0
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_ogt_int_n0_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp ogt float %f, -0.0
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_ole_int_0_uitofp(
 ; CHECK: icmp eq i32 %i, 0

 ; CHECK: fcmp oeq
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_0_uitofp_ppcf128(i32 %i) {
   %f = uitofp i32 %i to ppc_fp128
   %cmp = fcmp oeq ppc_fp128 %f, 0xM00000000000000000000000000000000
   ret i1 %cmp
 }
 
-; CHECK-LABEL: @i32_cast_cmp_oeq_int_i24max_uitofp(
-; CHECK: uitofp
-; CHECK: fcmp oeq
+; Since 0xFFFFFF fits in a float, and one less and 
+; one more than it also fits without rounding, the 
+; test can be optimized to an integer compare.
 
-; XCHECK: icmp eq i32 %i, 16777215
-; XCHECK-NEXT: ret
+; CHECK-LABEL: @i32_cast_cmp_oeq_int_i24max_uitofp(
+; CHECK: icmp eq i32 %i, 16777215
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_i24max_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0x416FFFFFE0000000
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_i24max_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp oeq
-
-; XCHECK: icmp eq i32 %i, 16777215
-; XCHECK-NEXT: ret
+; CHECK: icmp eq i32 %i, 16777215
+; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_i24max_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0x416FFFFFE0000000
   ret i1 %cmp
 }
 
+; Though 0x1000000 fits in a float, one more than it 
+; would round to it too, hence a single integer comparison 
+; does not suffice.
+
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_i24maxp1_uitofp(
 ; CHECK: uitofp
 ; CHECK: fcmp oeq
 
 ; XCHECK: icmp eq i32 %i, 16777216
 ; XCHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_i24maxp1_uitofp(i32 %i) {
   %f = uitofp i32 %i to float

 ; CHECK: fcmp oeq
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_i32umax_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0x41F0000000000000
   ret i1 %cmp
 }
 
+; 32-bit unsigned integer cannot possibly round up to 1<<33
+; CHECK-LABEL: @i32_cast_cmp_oeq_int_big_uitofp(
+; CHECK-NEXT: ret i1 false
+define i1 @i32_cast_cmp_oeq_int_big_uitofp(i32 %i) {
+  %f = uitofp i32 %i to float
+  %cmp = fcmp oeq float %f, 0x4200000000000000
+  ret i1 %cmp
+}
+
+; 32-bit signed integer cannot possibly round up to 1<<32
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_i32umax_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp oeq
-; CHECK-NEXT: ret
+; CHECK-NEXT: ret i1 false
 define i1 @i32_cast_cmp_oeq_int_i32umax_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0x41F0000000000000
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_i32imin_uitofp(
 ; CHECK: uitofp

 ; CHECK: fcmp oeq
 ; CHECK-NEXT: ret
 define i1 @i32_cast_cmp_oeq_int_negi32umax_uitofp(i32 %i) {
   %f = uitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0xC1F0000000000000
   ret i1 %cmp
 }
 
+; 32-bit signed integer cannot possibly round to -1<<32
 ; CHECK-LABEL: @i32_cast_cmp_oeq_int_negi32umax_sitofp(
-; CHECK: sitofp
-; CHECK: fcmp oeq
-; CHECK-NEXT: ret
+; CHECK-NEXT: ret i1 false
 define i1 @i32_cast_cmp_oeq_int_negi32umax_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp oeq float %f, 0xC1F0000000000000
   ret i1 %cmp
 }
 
 ; CHECK-LABEL: @i32_cast_cmp_oeq_half_uitofp(
 ; CHECK: ret i1 false

 
 ; CHECK-LABEL: @i32_cast_cmp_une_half_sitofp(
 ; CHECK: ret i1 true
 define i1 @i32_cast_cmp_une_half_sitofp(i32 %i) {
   %f = sitofp i32 %i to float
   %cmp = fcmp une float %f, 0.5
   ret i1 %cmp
 }
+
+; CHECK-LABEL: @i32_cast_cmp_oeq_int_inf_uitofp(
+; CHECK-NEXT: ret i1 false
+define i1 @i32_cast_cmp_oeq_int_inf_uitofp(i32 %i) {
+  %f = uitofp i32 %i to float
+  %cmp = fcmp oeq float %f, 0x7FF0000000000000
+  ret i1 %cmp
+}
+
+; CHECK-LABEL: @i32_cast_cmp_oeq_int_inf_sitofp(
+; CHECK-NEXT: ret i1 false
+define i1 @i32_cast_cmp_oeq_int_inf_sitofp(i32 %i) {
+  %f = sitofp i32 %i to float
+  %cmp = fcmp oeq float %f, 0x7FF0000000000000
+  ret i1 %cmp
+}
+
+; An i128 could round to an IEEE single-precision infinity.
+; CHECK-LABEL: @i128_cast_cmp_oeq_int_inf_uitofp(
+; CHECK: uitofp
+; CHECK: fcmp oeq
+; CHECK-NEXT: ret
+define i1 @i128_cast_cmp_oeq_int_inf_uitofp(i128 %i) {
+  %f = uitofp i128 %i to float
+  %cmp = fcmp oeq float %f, 0x7FF0000000000000
+  ret i1 %cmp
+}