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

[John McCall]

On Jun 20, 2011, at 12:29 AM, Duncan Sands wrote:

> Hi John,
> 
>> On Jun 19, 2011, at 5:58 AM, Duncan Sands wrote:
>>>> +  // Insist that the amount-to-allocate not overflow.
>>>> +  OverflowingBinaryOperator *OBI = dyn_cast<OverflowingBinaryOperator>(Val);
>>>> +  if (OBI&&   !OBI->hasNoUnsignedWrap()) return 0;
>>> 
>>> I noticed you dropped the test for NSW.  I thought this was a problem because
>>> front-ends generally don't ever produce NUW, though they may produce NSW...
>> 
>> Stuart asked me to look at this, and I gave him a quick answer
>> that it's generally correct for nuw arithmetic, but that I wasn't as
>> certain for nsw arithmetic.  Now I think it's probably fine for either
>> as long as it's homogenous throughout.
>> 
>> The main restriction on the alloca size argument can be expressed as:
>>  (N * sizeof(T)) udiv sizeof(T) == N
>> 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.  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
 - either the input or output to that multiply is negative.
This ends up being equivalent to saying that it's nuw nsw and cannot have a
negative result.  Since transforms are not permitted to introduce undefined
behavior, that means that that this transform has to be forbidden unless we
can prove that the multiply we're factoring out will not cause a signed or
unsigned overflow, under the assumption that the end result doesn't end up
being negative.

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.

> I think I saw some GCC testcases fly by in which people were allocating stack
> objects larger than 2^32, so I'm not sure that "N * sizeof(T) < 2^31" is a
> reasonable assumption...

Okay, so the semantics should be in terms of sizeof(sp).  I'm fine with that.

John.