[llvm-commits] InstCombining, small bug alloca + bitcast

[Stepan Dyatkovskiy]

Hello Duncan,

Look at string:
if (OBI && !OBI->hasNoUnsignedWrap())
That's original string that was before my patch.

I had replaced it with
if (OBI && !OBI->hasNoUnsignedWrap() && !OBI->hasNoSignedWrap())

You think that this string really should be commented?

As I understood, here I supposed that nuw and nsw gives guarantee that 
value will not exceed MAX_INT and MAX_UINT respectively, since in 
another case it will poison value. And the value also will not wrapped 
from MAX_UINT to 0. So it is assumed that result will always linearly 
depended from arguments. It means that we can reduce case
%7 = add nsw i32 %6, 2048
%8 = alloca i8, i32 %7
%9 = bitcast i8* %8 to double*

to the single "alloca double, i32 %recalculated_size " instruction.

The same with "nuw" attr.

So I just added case for nsw, since probably author of original string 
forgot to do it.

-Stepan.

Duncan Sands wrote:
> Hi Stepan,
>
>> I also added some usefull checks to test and fixed InstCombineCasts.cpp.
>
> what was wrong with InstCombineCasts?  In fact the existing check
>
>       // Cannot look past anything that might overflow.
>       OverflowingBinaryOperator *OBI = dyn_cast<OverflowingBinaryOperator>(Val);
>       if (OBI&&  !OBI->hasNoUnsignedWrap()) {
>         Scale = 1;
>         Offset = 0;
>         return Val;
>       }
>
> looks bogus to me: without it, you correctly get Scale and Offset such that
> Val = X*Scale + Offset, which is what the routine is supposed to do according
> to the comment describing it.
>
> So if this check is really needed, it must be because of some undocumented
> semantics in how the routine is supposed to behave.  What are these semantics?
> They should be documented.  Once it is clear what this routine is supposed to
> be doing, it should become clear whether your change is correct or not.
>
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits