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

[Steve Canon via llvm-commits]

scanon added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1426
@@ -1424,1 +1425,3 @@
+// least significant set bits of abs(X) fits into the mantissa, and that the
+// number of trailing zeros fits into the exponent.
 Instruction *InstCombiner::FoldItoFPtoI(Instruction &FI) {
----------------
A few things:

1. This:

> and that the number of trailing zeros fits into the exponent

is really confused.  That's not how it [floating-point encoding] works at all.  A correct algorithm for determining if an integer n can be exactly represented is (pseudocode):
```
m = absoluteValue(n)
i = mostSignificantNonzeroBit(m)
j = leastSignificantNonzeroBit(m)
if (i - j < numberOfSignificandBits) {
  // significand is representable, check exponent
  if (i <= greatestFiniteExponent) {
    // number can be exactly represented
  }
}
// number cannot be exactly represented
```
In particular, if we look at your example, `(2 ** 24 - 1) << 10`, this number absolutely is exactly representable in single-precision.  We have:
```
m = 2**24-1 << 10
i = 33
j = 10
i-j = 23 < numberOfSignificandBits = 24
i = 33 <= greatestFiniteExponent = 127.
```

2. Please don't use the term "mantissa".  It is frequently used, even in the LLVM sources, but it is subtly wrong (https://en.wikipedia.org/wiki/Significand#Use_of_.22mantissa.22).  The correct term is "significand"; any new written code should use that instead.


http://reviews.llvm.org/D19178