[PATCH] D19178: Broaden FoldItoFPtoI to try and establish whether the integer value fits into the float type

[Eli Friedman via llvm-commits]

eli.friedman added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1496
@@ +1495,3 @@
+                MostSignificantPossiblySetBit = std::max(HighestNotKnownZero,
+                                                         LowestNotKnownOne);
+            }
----------------
aarzee wrote:
> eli.friedman wrote:
> > If a value has unknown sign, isn't KnownZero.countLeadingOnes() always going to zero?
> I believe that's correct, yes. Thanks for catching that.
> 
> This means:
>   - The absolute value of an integer with unknown sign and bitwidth X has a maximum bitwidth X;
>   - We know that we already checked whether the width of the integer fits into the significand earlier in the function, and if we reached this point in the function that comparison was false;
>   - So if we reach this point in the function with unknown sign, we can immediately return nullptr.
> 
> We can add this line after the definitions of KnownNegative and PossiblyNegative, and remove the respective else blocks later in the function:
> 
> 
> ```
> if (PossiblyNegative && !KnownNegative)
>   // If the sign of the number is unknown, the bit range will be the width of the integer type,
>   // and we already checked whether that will fit into the fp type earlier in the function, so
>   // we can exit early.
>   return nullptr;
> ```
> 
> I'll do this in the next revision.
> 
> 
I think you can still optimize based on trailing zero bits even if the sign isn't known.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1507
@@ +1506,3 @@
+    Safe = BitRange <= SignificandWidth && MostSignificantPossiblySetBit <
+      APFloat::semanticsMaxExponent(OpITy->getFltSemantics());
+  }
----------------
aarzee wrote:
> eli.friedman wrote:
> > Thinking about it a bit more, we don't actually care if the exponent overflows; the end result would be undef, so it doesn't matter.
> Is this explicitly defined somewhere?
http://llvm.org/docs/LangRef.html#fptosi-to-instruction .  "If the value cannot fit in ty2, the results are undefined."


https://reviews.llvm.org/D19178