[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 a comment.

Needs more comments in the code, and more tests.  In particular, you have zero tests of values with trailing zeros.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1467
@@ +1466,3 @@
+    LeastSignificantPossiblySetBit = std::max(
+                                        int(KnownZero.countTrailingOnes()) - 1,
+                                        0);
----------------
Why are you subtracting one here?  The use of std::max here is suspicious; you should be treating UUUUUUUU and UUUUUUU0 differently.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1490
@@ +1489,3 @@
+                MostSignificantPossiblySetBit = std::max(HighestNotKnownZero,
+                                                         LowestNotKnownOne);
+            }
----------------
I suspect what you're doing here is wrong, but it's hard to follow without any comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1497
@@ +1496,3 @@
+    Safe = BitRange <= SignificandWidth && MostSignificantPossiblySetBit <=
+      (1 << ((int)OpITy->getScalarSizeInBits() - SignificandWidth - 1)) - 1;
+  }
----------------
What are you trying to check here?  (Maybe you're looking for APFloat::semanticsMaxExponent?)


https://reviews.llvm.org/D19178