[llvm-commits] [llvm] r132926 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineCasts.cpp test/Transforms/InstCombine/2011-06-13-nsw-alloca.ll

[Stuart Hastings]

On Jun 13, 2011, at 12:57 PM, Duncan Sands wrote:

> Hi Stuart,
> 
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Mon Jun 13 13:48:49 2011
>> @@ -71,6 +71,11 @@
>>    // This requires TargetData to get the alloca alignment and size information.
>>    if (!TD) return 0;
>> 
>> +  // Insist that the amount-to-allocate not overflow.
>> +  OverflowingBinaryOperator *OBI =
>> +    dyn_cast<OverflowingBinaryOperator>(AI.getOperand(0));
>> +  if (OBI&&  !(OBI->hasNoSignedWrap() || OBI->hasNoUnsignedWrap())) return 0;
> 
> is it correct to allow both nsw and nuw here?  Is the amount signed or unsigned?

Sort of.  Clang actually generates nsw for some alloca parameters, but that doesn't make sense to me; alloca properly takes an unsigned quantity.

Accepting nsw and nuw here allows InstCombine to do this transformation safely; both indicate no wraparound.  Before my patch, InstCombine didn't check for either of these, and we found a test case that would break.

I'll take this up with the Clang folks.

stuart