[llvm-commits] [llvm] r128731 - in /llvm/trunk/lib/Transforms/InstCombine: InstCombine.h InstCombineCasts.cpp

[Benjamin Kramer]

On 01.04.2011, at 23:51, Frits van Bommel wrote:

> I know you only moved this code, but I still have some comments on it:
> 
> On Fri, Apr 1, 2011 at 10:09 PM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> +    // (x <s 0) ? -1 : 0 -> ashr x, 31   -> all ones if signed
>> +    // (x >s -1) ? -1 : 0 -> ashr x, 31  -> all ones if not signed
> 
> The second comment line is incorrect. The ashr should be 'not (ashr x, 31)'.

Fixed in r128745.

>> +    if ((Pred == ICmpInst::ICMP_SLT && Op1C->isZero()) ||
>> +        (Pred == ICmpInst::ICMP_SGT && Op1C->isAllOnesValue())) {
> 
> It seems to me that this can also be done for (x <u signbit) and (x >u
> signed_max). Unless those are canonicalized to signed comparisons
> elsewhere?

Yeah, these two are the cases that the many possible checks for a sign bit
are canonicalized into. Thanks for the great and thorough reviews :)