[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
Eli Friedman
eli.friedman at gmail.com
Mon Dec 10 14:16:12 PST 2012
On Mon, Dec 10, 2012 at 2:03 PM, Manman Ren <mren at apple.com> wrote:
>
> On Dec 10, 2012, at 1:38 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>
>> On Mon, Dec 3, 2012 at 4:52 PM, Manman Ren <mren at apple.com> wrote:
>>> Author: mren
>>> Date: Mon Dec 3 18:52:33 2012
>>> New Revision: 169197
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=169197&view=rev
>>> Log:
>>> 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.
>>>
>>> rdar://12713765
>>>
>>> Added:
>>> llvm/trunk/test/CodeGen/ARM/alloc-no-stack-realign.ll
>>> Modified:
>>> llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h
>>> llvm/trunk/include/llvm/Target/TargetFrameLowering.h
>>> llvm/trunk/lib/CodeGen/MachineFunction.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h?rev=169197&r1=169196&r2=169197&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h (original)
>>> +++ llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h Mon Dec 3 18:52:33 2012
>>> @@ -221,8 +221,11 @@
>>> /// just allocate them normally.
>>> bool UseLocalStackAllocationBlock;
>>>
>>> + /// Whether the "realign-stack" option is on.
>>> + bool RealignOption;
>>> public:
>>> - explicit MachineFrameInfo(const TargetFrameLowering &tfi) : TFI(tfi) {
>>> + explicit MachineFrameInfo(const TargetFrameLowering &tfi, bool RealignOpt)
>>> + : TFI(tfi), RealignOption(RealignOpt) {
>>> StackSize = NumFixedObjects = OffsetAdjustment = MaxAlignment = 0;
>>> HasVarSizedObjects = false;
>>> FrameAddressTaken = false;
>>>
>>> Modified: llvm/trunk/include/llvm/Target/TargetFrameLowering.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetFrameLowering.h?rev=169197&r1=169196&r2=169197&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Target/TargetFrameLowering.h (original)
>>> +++ llvm/trunk/include/llvm/Target/TargetFrameLowering.h Mon Dec 3 18:52:33 2012
>>> @@ -47,11 +47,12 @@
>>> unsigned StackAlignment;
>>> unsigned TransientStackAlignment;
>>> int LocalAreaOffset;
>>> + bool StackRealignable;
>>> public:
>>> TargetFrameLowering(StackDirection D, unsigned StackAl, int LAO,
>>> - unsigned TransAl = 1)
>>> + unsigned TransAl = 1, bool StackReal = true)
>>> : StackDir(D), StackAlignment(StackAl), TransientStackAlignment(TransAl),
>>> - LocalAreaOffset(LAO) {}
>>> + LocalAreaOffset(LAO), StackRealignable(StackReal) {}
>>>
>>> virtual ~TargetFrameLowering();
>>>
>>> @@ -76,6 +77,12 @@
>>> return TransientStackAlignment;
>>> }
>>>
>>> + /// isStackRealignable - This method returns whether the stack can be
>>> + /// realigned.
>>> + bool isStackRealignable() const {
>>> + return StackRealignable;
>>> + }
>>> +
>>> /// getOffsetOfLocalArea - This method returns the offset of the local area
>>> /// from the stack pointer on entrance to a function.
>>> ///
>>>
>>> Modified: llvm/trunk/lib/CodeGen/MachineFunction.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineFunction.cpp?rev=169197&r1=169196&r2=169197&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/MachineFunction.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/MachineFunction.cpp Mon Dec 3 18:52:33 2012
>>> @@ -58,7 +58,8 @@
>>> else
>>> RegInfo = 0;
>>> MFInfo = 0;
>>> - FrameInfo = new (Allocator) MachineFrameInfo(*TM.getFrameLowering());
>>> + FrameInfo = new (Allocator) MachineFrameInfo(*TM.getFrameLowering(),
>>> + TM.Options.RealignStack);
>>> if (Fn->getFnAttributes().hasAttribute(Attributes::StackAlignment))
>>> FrameInfo->ensureMaxAlignment(Fn->getAttributes().
>>> getFnAttributes().getStackAlignment());
>>> @@ -448,15 +449,31 @@
>>> /// ensureMaxAlignment - Make sure the function is at least Align bytes
>>> /// aligned.
>>> void MachineFrameInfo::ensureMaxAlignment(unsigned Align) {
>>> + if (!TFI.isStackRealignable() || !RealignOption)
>>> + assert(Align <= TFI.getStackAlignment() &&
>>> + "For targets without stack realignment, Align is out of limit!");
>>> if (MaxAlignment < Align) MaxAlignment = Align;
>>> }
>>>
>>> +/// clampStackAlignment - Clamp the alignment if requested and emit a warning.
>>> +static inline unsigned clampStackAlignment(bool ShouldClamp, unsigned Align,
>>> + unsigned StackAlign) {
>>> + if (!ShouldClamp || Align <= StackAlign)
>>> + return Align;
>>> + DEBUG(dbgs() << "Warning: requested alignment " << Align
>>> + << " exceeds the stack alignment " << StackAlign
>>> + << " when stack realignment is off" << '\n');
>>> + return StackAlign;
>>> +}
>>> +
>>> /// CreateStackObject - Create a new statically sized stack object, returning
>>> /// a nonnegative identifier to represent it.
>>> ///
>>> int MachineFrameInfo::CreateStackObject(uint64_t Size, unsigned Alignment,
>>> bool isSS, bool MayNeedSP, const AllocaInst *Alloca) {
>>> assert(Size != 0 && "Cannot allocate zero size stack objects!");
>>> + Alignment = clampStackAlignment(!TFI.isStackRealignable() || !RealignOption,
>>> + Alignment, TFI.getStackAlignment());
>>> Objects.push_back(StackObject(Size, Alignment, 0, false, isSS, MayNeedSP,
>>> Alloca));
>>> int Index = (int)Objects.size() - NumFixedObjects - 1;
>>> @@ -471,6 +488,8 @@
>>> ///
>>> int MachineFrameInfo::CreateSpillStackObject(uint64_t Size,
>>> unsigned Alignment) {
>>> + Alignment = clampStackAlignment(!TFI.isStackRealignable() || !RealignOption,
>>> + Alignment, TFI.getStackAlignment());
>>> CreateStackObject(Size, Alignment, true, false);
>>> int Index = (int)Objects.size() - NumFixedObjects - 1;
>>> ensureMaxAlignment(Alignment);
>>
>> Isn't this just going to cause a different miscompile if the alignment
>> of the stack object actually matters? It should be a fatal error to
>> try and create a stack slot we can't sufficiently align.
>
> This patch will emit a warning if the alignment is actually clamped.
> It is hard to say whether we should emit a warning or throw a fatal error.
IMO, it should be an error; users will never see the message otherwise.
> 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.
-Eli
More information about the llvm-commits
mailing list