Standardize how we check for legality of frame realignment

[reames]

Summary

We currently have three ways to check whether a stack can be realigned. TargetFrameInfo, MachineFrameInfo, and TargetRegisterInfo. This patch attempts to standardize on one of them (TRI) as the source of truth.

This patch does change behaviour in corner-cases. Previously, an overaligned static alloc in a no-realign-stack function had it's alignment silently ignored. Now, the frame isn't stack object isn't dynamically aligned on entry, but instead becomes a variable sized frame around the alloca scope. (i.e. it's treated as a dynamic alloca)

[reames]

Highlight the change in behaviour for over aligned allocas w/o stack realignment.

The result is a bit counter intuitive. We generate a variable size frame, but not one with realignment. Not really sure this is any better, but at least it's consistent with frames which aren't realignable as a target property?

[ebrevnov]

I was independently looking to this part and came up with essentially same change. The only difference is that I chose MachineFrameInfo as source of "truth". Not really sure which one is better though...

[reames]

ping

[reames]

Abandoning an old review I'm not going to return to any time soon.

include/llvm/CodeGen/TargetFrameLowering.h

public:
   /// which the stack pointer must be aligned at all times, even between
   /// calls.
   ///
   unsigned getTransientStackAlignment() const {
     return TransientStackAlignment;
   }
 
   /// isStackRealignable - This method returns whether the stack can be
-  /// realigned.
+  /// realigned on this target.  Note that even for targets which allow stack
+  /// realignment, not all functions are required to.  Most potential callers
+  /// probably want TargetRegisterInfo::canRealignStack(MF) instead.
   bool isStackRealignable() const {
     return StackRealignable;
   }
 
   /// Return the skew that has to be applied to stack alignment under
   /// certain conditions (e.g. stack was adjusted before function \p MF
   /// was called).
   virtual unsigned getStackAlignmentSkew(const MachineFunction &MF) const;

lib/CodeGen/MachineFunction.cpp

void MachineFunction::init() {
   Properties.set(MachineFunctionProperties::Property::IsSSA);
   Properties.set(MachineFunctionProperties::Property::TracksLiveness);
   if (STI->getRegisterInfo())
     RegInfo = new (Allocator) MachineRegisterInfo(this);
   else
     RegInfo = nullptr;
 
   MFInfo = nullptr;
-  // We can realign the stack if the target supports it and the user hasn't
-  // explicitly asked us not to.
-  bool CanRealignSP = STI->getFrameLowering()->isStackRealignable() &&
-                      !F.hasFnAttribute("no-realign-stack");
+
+  bool CanRealignSP = STI->getRegisterInfo()->canRealignStack(*this);
   FrameInfo = new (Allocator) MachineFrameInfo(
       getFnStackAlignment(STI, F), /*StackRealignable=*/CanRealignSP,
       /*ForceRealign=*/CanRealignSP &&
           F.hasFnAttribute(Attribute::StackAlignment));
 
   if (F.hasFnAttribute(Attribute::StackAlignment))
     FrameInfo->ensureMaxAlignment(F.getFnStackAlignment());
 

lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp

 
 void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
                                SelectionDAG *DAG) {
   Fn = &fn;
   MF = &mf;
   TLI = MF->getSubtarget().getTargetLowering();
   RegInfo = &MF->getRegInfo();
   const TargetFrameLowering *TFI = MF->getSubtarget().getFrameLowering();
+  const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
   unsigned StackAlign = TFI->getStackAlignment();
   DA = DAG->getDivergenceAnalysis();
 
   // Check whether the function can return without sret-demotion.
   SmallVector<ISD::OutputArg, 4> Outs;
   CallingConv::ID CC = Fn->getCallingConv();
 
   GetReturnInfo(CC, Fn->getReturnType(), Fn->getAttributes(), Outs, *TLI,
for (const Instruction &I : BB) {
         unsigned Align =
           std::max((unsigned)MF->getDataLayout().getPrefTypeAlignment(Ty),
                    AI->getAlignment());
 
         // Static allocas can be folded into the initial stack frame
         // adjustment. For targets that don't realign the stack, don't
         // do this if there is an extra alignment requirement.
         if (AI->isStaticAlloca() &&
-            (TFI->isStackRealignable() || (Align <= StackAlign))) {
+            (TRI->canRealignStack(mf) || (Align <= StackAlign))) {
           const ConstantInt *CUI = cast<ConstantInt>(AI->getArraySize());
           uint64_t TySize = MF->getDataLayout().getTypeAllocSize(Ty);
 
           TySize *= CUI->getZExtValue();   // Get total allocated size.
           if (TySize == 0) TySize = 1; // Don't create zero-sized stack objects.
           int FrameIndex = INT_MAX;
           auto Iter = CatchObjects.find(AI);
           if (Iter != CatchObjects.end() && TLI->needsFixedCatchObjects()) {
for (const Instruction &I : BB) {
         }
       }
 
       // Look for inline asm that clobbers the SP register.
       if (isa<CallInst>(I) || isa<InvokeInst>(I)) {
         ImmutableCallSite CS(&I);
         if (isa<InlineAsm>(CS.getCalledValue())) {
           unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
-          const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
           std::vector<TargetLowering::AsmOperandInfo> Ops =
               TLI->ParseConstraints(Fn->getParent()->getDataLayout(), TRI, CS);
           for (TargetLowering::AsmOperandInfo &Op : Ops) {
             if (Op.Type == InlineAsm::isClobber) {
               // Clobbers don't have SDValue operands, hence SDValue().
               TLI->ComputeConstraintToUse(Op, SDValue(), DAG);
               std::pair<unsigned, const TargetRegisterClass *> PhysReg =
                   TLI->getRegForInlineAsmConstraint(TRI, Op.ConstraintCode,

lib/CodeGen/TargetRegisterInfo.cpp

for (auto Reg : Hints_MRI.second) {
 
     // All clear, tell the register allocator to prefer this register.
     Hints.push_back(Phys);
   }
   return false;
 }
 
 bool TargetRegisterInfo::canRealignStack(const MachineFunction &MF) const {
-  return !MF.getFunction().hasFnAttribute("no-realign-stack");
+  return MF.getSubtarget().getFrameLowering()->isStackRealignable() &&
+    !MF.getFunction().hasFnAttribute("no-realign-stack");
 }
 
 bool TargetRegisterInfo::needsStackRealignment(
     const MachineFunction &MF) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
   const Function &F = MF.getFunction();
   unsigned StackAlign = TFI->getStackAlignment();

test/CodeGen/X86/alloca-overaligned.ll

; CHECK-NEXT:    retq
   %a = alloca i64, align 64
   call void @capture(i64* %a)
   ret void
 }
 
 define void @test_norealign() "no-realign-stack" {
 ; CHECK-LABEL: test_norealign:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    pushq %rbp
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    .cfi_offset %rbp, -16
+; CHECK-NEXT:    movq %rsp, %rbp
+; CHECK-NEXT:    .cfi_def_cfa_register %rbp
 ; CHECK-NEXT:    movq %rsp, %rdi
+; CHECK-NEXT:    addq $-16, %rdi
+; CHECK-NEXT:    andq $-64, %rdi
+; CHECK-NEXT:    movq %rdi, %rsp
 ; CHECK-NEXT:    callq capture
-; CHECK-NEXT:    popq %rax
-; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    movq %rbp, %rsp
+; CHECK-NEXT:    popq %rbp
+; CHECK-NEXT:    .cfi_def_cfa %rsp, 8
 ; CHECK-NEXT:    retq
   %a = alloca i64, align 64
   call void @capture(i64* %a)
   ret void
 }