[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]

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.

-Eli