[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:1496
@@ +1495,3 @@
+                MostSignificantPossiblySetBit = std::max(HighestNotKnownZero,
+                                                         LowestNotKnownOne);
+            }
----------------
eli.friedman wrote:
> aarzee wrote:
> > eli.friedman wrote:
> > > 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.
> > Again, you're right. I wasn't thinking about the least significant possibly set bit. How about this:
> > 
> > 
> > ```
> > // If the number is known to be negative, the highest possibly set bit
> > // is the lowest bit not known to be one.
> > // Otherwise, it's the highest bit not known to be zero.
> > 
> > if (KnownNegative)
> >   MostSignificantPossiblySetBit = int(KnownOne.countTrailingOnes()); 
> > else
> >   MostSignificantPossiblySetBit = int(BitWidth - KnownZero.countLeadingOnes()) - 1;
> > ```
> I assume you meant to use countLeadingOnes for both?  Looks roughly right.
Nope. Lowest bit not known to be one is KnownOne.countTrailingOnes, and highest bit not known to be zero is BitWidth - KnownZero.countLeadingOnes() - 1.


https://reviews.llvm.org/D19178