[llvm-commits] [llvm] r133285 - /llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp

[Duncan Sands]

Hi John,

>>> However, there's a second restriction, which is that it's undefined
>>> behavior to allocate something too large for the stack.  Technically
>>> this is target-specific, but if we just assume by fiat that 2^31 is too large,
>>> then as long as the arithmetic is being done in some reasonable size
>>> then we know that N * sizeof(T) isn't permitted to signed overflow, either.
>>
>> if you are willing to assume that then you don't need to check for nuw either...
>
> We can't *assume* it doesn't overflow.

I thought *you* were suggesting that we assume no overflow when you said "but
if we just assume by fiat that 2^31 is too large, then as long as the arithmetic
is being done in some reasonable size then we know that N * sizeof(T) isn't
permitted to signed overflow, either."  I guess I misunderstood.  I'm still not
sure what you were trying to say :)

   That's the whole point of this discussion.
> There's an implicit multiply in sizeof(sp) as part of the alloca instruction, and
> it's undefined behavior if either
>    - that multiply causes unsigned overflow or

Sure.

>    - either the input or output to that multiply is negative.

Why?  Are you saying that an alloca of 2^31 i8's should be considered to result
in undefined behaviour?  And what about this: alloca i8, i8 128

By the way, on a 64 bit machine I guess it costs nothing to change the type of
the alloca numelements parameter to i64, so if Stuart's testcase was on such a
platform then it could become: %c = alloca [16 x i8], i64 %N
where %N = zext i32 %n to i64.

> Going back to Stuart's original test case, we can't apply the transform to this:
>    %a = mul i32 16, %n
>    %b = alloca i8, i32 %b
>    %c = bitcast i8* %x to [16 x i8]*
> as that might introduce undefined behavior, because the multiply in %a is
> allowed to overflow, and the multiply in this:
>    %c = alloca [16 x i8], i32 %n
> is not.

Indeed.

Ciao, Duncan.