[llvm-commits] [PATCH] Stack Alignment: clamp the alignment of stack objects in MachineFrameInfo
Manman Ren
mren at apple.com
Wed Jan 9 17:12:30 PST 2013
In r172027.
Thanks,
Manman
On Jan 8, 2013, at 5:16 PM, Manman Ren wrote:
>
> Thanks for reviewing, the return value can be 0, that is why nonnegative was used :)
>
> Manman
>
> On Jan 7, 2013, at 6:12 PM, Evan Cheng wrote:
>
>> LGTM. One nitpick: nonnegative -> positive. :-)
>>
>> Evan
>>
>> On Dec 21, 2012, at 1:19 PM, Manman Ren <mren at apple.com> wrote:
>>
>>> Ping
>>> <emit_error.patch>
>>>
>>> On Dec 20, 2012, at 8:02 AM, Manman Ren wrote:
>>>
>>>> Ping
>>>>
>>>> On Dec 18, 2012, at 4:13 PM, Manman Ren <mren at apple.com> wrote:
>>>>
>>>>>
>>>>> Stack Alignment: throw error if we can't satisfy the minimal alignment
>>>>> requirement when creating stack objects in MachineFrameInfo.
>>>>>
>>>>> Add CreateStackObjectWithMinAlign to throw error when the minimal alignment
>>>>> can't be achieved and to clamp the alignment when the preferred alignment
>>>>> can't be achieved. Same is true for CreateVariableSizedObject.
>>>>> Will not emit error in CreateSpillStackObject or CreateStackObject.
>>>>>
>>>>> As long as callers of CreateStackObject do not assume the object will be
>>>>> aligned at the requested alignment, we should not have miscompile since
>>>>> later optimizations looking at the object's alignment will have the correct
>>>>> information.
>>>>>
>>>>>
>>>>> <emit_error.patch>
>>>>>
>>>>> Thanks,
>>>>> Manman
>>>>>
>>>>> On Dec 18, 2012, at 12:47 PM, Bob Wilson wrote:
>>>>>
>>>>>>
>>>>>> On Dec 18, 2012, at 11:27 AM, Manman Ren <mren at apple.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> There are some discussions in thread
>>>>>>> Re: [llvm-commits] [llvm] r169197 - in /llvm/trunk: include/llvm/CodeGen/MachineFrameInfo.h include/llvm/Target/TargetFrameLowering.h lib/CodeGen/MachineFunction.cpp test/CodeGen/ARM/alloc-no-stack-realign.ll
>>>>>>
>>>>>> As you can tell, I'm way behind on patch reviews… Sorry that I missed the other discussion.
>>>>>>
>>>>>> To Chris's comment about LLVMContext::emitError: I'm embarrassed to admit that I'd forgotten that was available for general error messages. I knew it was there for inline assembly, but didn't make the connection when I saw InstCombine writing a warning directly to errs(). I'll fix that and check for other places that may also need to be updated.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>> When creating stack object, we use Align
>>>>>>>>> Align = std::max((unsigned)TLI.getDataLayout()->getPrefTypeAlignment(Ty),
>>>>>>>>> AI->getAlignment());
>>>>>>>>> The preferred alignment may be too big for the target, and it makes sense to give a warning instead of a fatal error.
>>>>>>>>
>>>>>>>> If we can't realign the stack, we should print an error if
>>>>>>>> AI->getAlignment() is too large and simply ignore the return of
>>>>>>>> getPrefTypeAlignment if it's too large.
>>>>>>>
>>>>>>> This means going through the call sites of these functions CreateStackObject … and checking the alignment there instead of
>>>>>>> wrapping it up in these functions.
>>>>>>>
>>>>>>> Another option is to pass in an extra argument for the minimal alignment that must be satisfied.
>>>>>>>
>>>>>>> //////////////////////////////////////////////
>>>>>>> --> I will pass an extra argument for the minimal alignment and will use emitError if the minimal alignment can't be satisfied and
>>>>>>> will ignore the case where getPrefTypeAlignment is too large.
>>>>>>> --> If that does not sound right, please let me know,
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Manman
>>>>>>>
>>>>>>> On Dec 18, 2012, at 11:15 AM, Chris Lattner <clattner at apple.com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Dec 18, 2012, at 11:06 AM, Bob Wilson <bob.wilson at apple.com> wrote:
>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 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>).
>>>>>>>>
>>>>>>>> Backend errors like this should be reported with LLVMContext::emitError.
>>>>>>>
>>>>>>>>
>>>>>>>> -Chris
>>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130109/22a1f149/attachment.html>
More information about the llvm-commits
mailing list