[CodeGen] Don't assume that fixed stack objects are aligned in a stack-realigned function.

[cdavis5x]

Summary

After we make the adjustment, we can assume that for local allocas, but
not for stack parameters, the return address, or any other fixed stack
object (which has a negative offset and therefore lies prior to the
adjusted SP).

Fixes PR26662.

[cdavis5x]

Ping.

[cdavis5x]

Ping...

[cdavis5x]

Ping^3.

[rnk]

lgtm

lib/CodeGen/MachineFunction.cpp

87–88 ↗

(On Diff #51630)

As a separate issue, I noticed that StackRealignable and RealignOpt are redundant. We can get by with just StackRealignable, and making that be STI->getFrameLowering()->isStackRealignable() && F->hasFnAttribute("no-realign-stack").

[cdavis5x]

lib/CodeGen/MachineFunction.cpp

87–88 ↗

(On Diff #51630)

Yeah, but I think I'll save that for another patch.

[rnk]

lib/CodeGen/MachineFunction.cpp

87–88 ↗

(On Diff #51630)

I went ahead and did this in r265971, it was more of an idle thought while staring at the related code. Thanks!

llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h

class MachineFrameInfo {
   /// Whether the local object blob needs to be allocated together. If not,
   /// PEI should ignore the isPreAllocated flags on the stack objects and
   /// just allocate them normally.
   bool UseLocalStackAllocationBlock;
 
   /// Whether the "realign-stack" option is on.
   bool RealignOption;
 
+  /// Whether the function has the \c alignstack attribute.
+  bool ForcedRealign;
+
   /// True if the function dynamically adjusts the stack pointer through some
   /// opaque mechanism like inline assembly or Win32 EH.
   bool HasOpaqueSPAdjustment;
 
   /// True if the function contains operations which will lower down to
   /// instructions which manipulate the stack pointer.
   bool HasCopyImplyingStackAdjustment;
 
class MachineFrameInfo {
 
   /// Not null, if shrink-wrapping found a better place for the prologue.
   MachineBasicBlock *Save;
   /// Not null, if shrink-wrapping found a better place for the epilogue.
   MachineBasicBlock *Restore;
 
 public:
   explicit MachineFrameInfo(unsigned StackAlign, bool isStackRealign,
-                            bool RealignOpt)
+                            bool RealignOpt, bool ForceRealign)
       : StackAlignment(StackAlign), StackRealignable(isStackRealign),
-        RealignOption(RealignOpt) {
+        RealignOption(RealignOpt), ForcedRealign(ForceRealign) {
     StackSize = NumFixedObjects = OffsetAdjustment = MaxAlignment = 0;
     HasVarSizedObjects = false;
     FrameAddressTaken = false;
     ReturnAddressTaken = false;
     HasStackMap = false;
     HasPatchPoint = false;
     AdjustsStack = false;
     HasCalls = false;

llvm/trunk/lib/CodeGen/MachineFunction.cpp

 
 // Out-of-line virtual method.
 MachineFunctionInfo::~MachineFunctionInfo() {}
 
 void ilist_traits<MachineBasicBlock>::deleteNode(MachineBasicBlock *MBB) {
   MBB->getParent()->DeleteMachineBasicBlock(MBB);
 }
 
+static inline unsigned getFnStackAlignment(const TargetSubtargetInfo *STI,
+                                           const Function *Fn) {
+  if (Fn->hasFnAttribute(Attribute::StackAlignment))
+    return Fn->getFnStackAlignment();
+  return STI->getFrameLowering()->getStackAlignment();
+}
+
 MachineFunction::MachineFunction(const Function *F, const TargetMachine &TM,
                                  unsigned FunctionNum, MachineModuleInfo &mmi)
     : Fn(F), Target(TM), STI(TM.getSubtargetImpl(*F)), Ctx(mmi.getContext()),
       MMI(mmi) {
   // Assume the function starts in SSA form.
   Properties.set(MachineFunctionProperties::Property::IsSSA);
   if (STI->getRegisterInfo())
     RegInfo = new (Allocator) MachineRegisterInfo(this);
   else
     RegInfo = nullptr;
 
   MFInfo = nullptr;
   FrameInfo = new (Allocator)
-      MachineFrameInfo(STI->getFrameLowering()->getStackAlignment(),
+      MachineFrameInfo(getFnStackAlignment(STI, Fn),
                        STI->getFrameLowering()->isStackRealignable(),
-                       !F->hasFnAttribute("no-realign-stack"));
+                       !F->hasFnAttribute("no-realign-stack"),
+                       !F->hasFnAttribute("no-realign-stack") &&
+                       F->hasFnAttribute(Attribute::StackAlignment));
 
   if (Fn->hasFnAttribute(Attribute::StackAlignment))
     FrameInfo->ensureMaxAlignment(Fn->getFnStackAlignment());
 
   ConstantPool = new (Allocator) MachineConstantPool(getDataLayout());
   Alignment = STI->getTargetLowering()->getMinFunctionAlignment();
 
   // FIXME: Shouldn't use pref alignment if explicit alignment is set on Fn.

 /// efficiency. By default, fixed objects are immutable. This returns an
 /// index with a negative value.
 int MachineFrameInfo::CreateFixedObject(uint64_t Size, int64_t SPOffset,
                                         bool Immutable, bool isAliased) {
   assert(Size != 0 && "Cannot allocate zero size fixed stack objects!");
   // The alignment of the frame index can be determined from its offset from
   // the incoming frame position.  If the frame object is at offset 32 and
   // the stack is guaranteed to be 16-byte aligned, then we know that the
-  // object is 16-byte aligned.
-  unsigned Align = MinAlign(SPOffset, StackAlignment);
+  // object is 16-byte aligned. Note that unlike the non-fixed case, if the
+  // stack needs realignment, we can't assume that the stack will in fact be
+  // aligned.
+  unsigned Align = MinAlign(SPOffset, ForcedRealign ? 1 : StackAlignment);
   Align = clampStackAlignment(!StackRealignable || !RealignOption, Align,
                               StackAlignment);
   Objects.insert(Objects.begin(), StackObject(Size, Align, SPOffset, Immutable,
                                               /*isSS*/   false,
                                               /*Alloca*/ nullptr, isAliased));
   return -++NumFixedObjects;
 }
 
 /// Create a spill slot at a fixed location on the stack.
 /// Returns an index with a negative value.
 int MachineFrameInfo::CreateFixedSpillStackObject(uint64_t Size,
                                                   int64_t SPOffset) {
-  unsigned Align = MinAlign(SPOffset, StackAlignment);
+  unsigned Align = MinAlign(SPOffset, ForcedRealign ? 1 : StackAlignment);
   Align = clampStackAlignment(!StackRealignable || !RealignOption, Align,
                               StackAlignment);
   Objects.insert(Objects.begin(), StackObject(Size, Align, SPOffset,
                                               /*Immutable*/ true,
                                               /*isSS*/ true,
                                               /*Alloca*/ nullptr,
                                               /*isAliased*/ false));
   return -++NumFixedObjects;

llvm/trunk/test/CodeGen/X86/stack-align.ll

entry:
   %buffer = alloca [2048 x i8], align 16
   %0 = ptrtoint [2048 x i8]* %buffer to i32
   %and = and i32 %0, -16
   ret i32 %and
 ; CHECK-LABEL: test4:
 ; CHECK-NOT: and
 ; CHECK: ret
 }
+
+%struct.sixteen = type { [16 x i8] }
+
+; Accessing stack parameters shouldn't assume stack alignment. Here we should
+; emit two 8-byte loads, followed by two 8-byte stores.
+define x86_stdcallcc void @test5(%struct.sixteen* byval nocapture readonly align 4 %s) #0 {
+  %d.sroa.0 = alloca [16 x i8], align 1
+  %1 = getelementptr inbounds [16 x i8], [16 x i8]* %d.sroa.0, i32 0, i32 0
+  call void @llvm.lifetime.start(i64 16, i8* %1)
+  %2 = getelementptr inbounds %struct.sixteen, %struct.sixteen* %s, i32 0, i32 0, i32 0
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %1, i8* %2, i32 16, i32 1, i1 true)
+  call void @llvm.lifetime.end(i64 16, i8* %1)
+  ret void
+; CHECK-LABEL: test5:
+; CHECK: and
+; CHECK: movsd
+; CHECK-NEXT: movsd
+; CHECK-NEXT: movsd
+; CHECK-NEXT: movsd
+}
+
+declare void @llvm.lifetime.start(i64, i8* nocapture) argmemonly nounwind
+
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture readonly, i32, i32, i1) argmemonly nounwind
+
+declare void @llvm.lifetime.end(i64, i8* nocapture) argmemonly nounwind
+
+attributes #0 = { nounwind alignstack=16 "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" }