[llvm-commits] [PATCH] Stack Alignment: clamp the alignment of stack objects in MachineFrameInfo

[Bob Wilson]

On Dec 3, 2012, at 12:41 PM, Manman Ren <mren at apple.com> wrote:

> 
> On Dec 2, 2012, at 10:58 PM, Bill Wendling <wendling at apple.com> wrote:
> 
>> On Nov 30, 2012, at 2:21 PM, Manman Ren <mren at apple.com> wrote:
>> 
>>>> On Nov 30, 2012, at 12:12 PM, Manman Ren <mren at apple.com> wrote:
>>>> 
>>>>> 
>>>>> Hi all,
>>>>> 
>>>>> Stack Alignment: when creating stack objects in MachineFrameInfo, make sure
>>>>> the alignment is clamped to TargetFrameLowering.getStackAlignment if the target
>>>>> does not support stack realignment or the option "realign-stack" is off.
>>>>> 
>>>>> This will cause miscompile if the address is treated as aligned and add is
>>>>> replaced with or in DAGCombine.
>>>>> 
>>>>> Added a bool StackRealignable to TargetFrameLowering to check whether stack
>>>>> realignment is implemented for the target. Also added a bool RealignOption
>>>>> to MachineFrameInfo to check whether the option "realign-stack" is on.
>>>>> 
>>>>> Moved a few functions from MachineFrameInfo.h to .cpp and added clamping of
>>>>> alignment to those functions.
>>>>> 
>> Hi Manman,
>> 
>> Could you move the functions from MachineFrameInfo.h first and then add the clamping code? Also, the debug output you emit should be within 'DEBUG()' macro. Like this:
> Hi Bill,
> Sure, I will submit it as two patches and guard with DEBUG.
>> 
>> 	DEBUG(dbgs() << "Warning: requested alignment " << Align
>> 	         << " exceeds the stack alignment " << StackAlign
>> 	         << " when stack realignment is off\n");
>> 
>> Or better yet make it an 'assert' if you don't expect this situation to ever occur:
> This can actually happen if the user specified a large alignment or the data type has a large alignment.

In that case, using DEBUG is absolutely the wrong thing.  The warning won't be produced in a compiler built without debugging.  Is there any way you could detect this in the front-end and produce a proper diagnostic?  (I haven't thought about that and haven't looked at the code, so forgive me if that's a stupid idea.)  Otherwise, you just have to print out the warning to "errs()".  In the long term, we need to figure out how to let the optimizer and code-gen produce real diagnostics (tracked at Apple in <rdar://problem/12867368>).