[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:1507
@@ +1506,3 @@
+    // then the conversion is undefined behavior.
+
+    Safe = BitRange <= SignificandWidth;
----------------
Are you sure subtracting one actually makes the most significant bit zero-indexed? Normally, finding the distance between two array indexes is just simple subtraction; the extra "+1" looks weird.  It's also kind of suspicious that your "zero-based" indexing can return a negative number (as an extreme case, if a 16-bit integer is all ones, your code says the most significant possibly set bit has index -1).

================
Comment at: test/Transforms/InstCombine/sitofp.ll:278
@@ +277,3 @@
+; the right of the sign bit.
+; (Bits of %B: UUUUUUUUUUUUUUUUUUUUUUUU0000000000, U = unknown)
+; CHECK-LABEL: test27
----------------
Maybe also add a corresponding test which doesn't fold?


https://reviews.llvm.org/D19178