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

[Carlos Liam via llvm-commits]

aarzee added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1507
@@ +1506,3 @@
+    // then the conversion is undefined behavior.
+
+    Safe = BitRange <= SignificandWidth;
----------------
eli.friedman wrote:
> aarzee wrote:
> > eli.friedman wrote:
> > > 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).
> > I'm going to take a look at the actual behavior, I think the least significant possibly set bit might be one-indexed; in any case something is behaving differently than I intended.
> > 
> > WRT the all-ones case, if a number has no unknown bits then it is by nature a constant, and would be handled by ConstProp, right?
> Yes, in the all-ones case it would get turned into a constant; not sure off the top of my head if that's guaranteed to happen before this code runs, though.
After checking I remembered the reason for the +1: what I'm calling BitRange is actually the //width//; for example, the width between the 1s in 0b101 would be 3. Maybe width isn't the best word. Whatever the case, this is the reason.


https://reviews.llvm.org/D19178