ARM: Compute MaxCallFrame size early

[MatzeB]

Summary

This avoids
ARMBaseRegisterInfo::canRealignStack()/ARMFrameLowering::hasReservedCallFrame()
giving different answers in early/late phases of codegen.

The attached testcase shows a particular nasty example where we would call TargetRegisterInfo::needsStackRealignment()->canRealignStack()->hasReservedCallFrame() and would end up not aligning the stack because hasReservedCallFrame() would report false in late passes only.

This exposes a method in MachineFrameInfo that calculates
MaxCallFrameSize and calls it after instruction selection to get the value computed early.

To make this work in GlobalISel this also:

• Move the TargetLoweringInfo::finalizeLowering code from IRTranslator to the end of InstructionSelect
• Changing the MachineVerifier to work when the reserved registers are not frozen yet (by using TargetRegisterInfo::getReservedRegs() instead of MachineRegisterInfo stuff).

[jmolloy]

lib/Target/ARM/ARMBaseRegisterInfo.cpp

253

Is it right that if we don't have accurate call frame information we assume FP is not used here? Shouldn't we default to the conservative "true"?

[MatzeB]

lib/Target/ARM/ARMBaseRegisterInfo.cpp

253

Yes until the end of instruction selection we assume FP is not used here. This only affects ScheduleDAGRRList. It should only be used to direct the DAG scheduling heuristic and not affect correctness. Anyway I don't see this making a difference one way or the other as the machine scheduler will reorder anyway.

So I'm fine either way, let's switch to ? TFI->hasFP(MF) : true; then.

[MatzeB]

lib/Target/ARM/ARMBaseRegisterInfo.cpp

253

And to put this into context: The change in this function is needed when we want to enable the assert in https://reviews.llvm.org/D32570.

[jmolloy]

OK, LGTM!

[MatzeB]

I had GlobalISel disabled in my earlier tests (cmake cache from times when it wasn't enabled yet).

I had to change GlobalISel to call TLI->finalizeLowering() late in InstructionSelect instead of after IRTranslator. While most ADJCALLSTACK/CALL instructions are already visible after IRTranslator we may miss operations getting implemented by library calls later. It also seems to delay freezing the reserved registers + extra adjustments until after ISel is finished. This also matches the current SelectionDAG behavior.

+Reviewers Ahmed because of the finalizeLowering/freezeReg movement.

[MatzeB]

In D32622#742404, @MatzeB wrote:

I had to change GlobalISel to call TLI->finalizeLowering() late in InstructionSelect instead of after IRTranslator. While most ADJCALLSTACK/CALL instructions are already visible after IRTranslator we may miss operations getting implemented by library calls later. It also seems to delay freezing the reserved registers + extra adjustments until after ISel is finished. This also matches the current SelectionDAG behavior.

+Reviewers Ahmed because of the finalizeLowering/freezeReg movement.

And some more testing reveals that some GlobalISel tests use the machineverifier which in turn requires frozen reserved registers at the moment :-/. I will see how to change that...

[MatzeB]

• Change MachineVerifier to work if reserved registers are not frozen yet.

include/llvm/CodeGen/MachineFrameInfo.h

public:
   /// Returns true if the function is variadic and contains a musttail call.
   bool hasMustTailInVarArgFunc() const { return HasMustTailInVarArgFunc; }
   void setHasMustTailInVarArgFunc(bool B) { HasMustTailInVarArgFunc = B; }
 
   /// Returns true if the function contains a tail call.
   bool hasTailCall() const { return HasTailCall; }
   void setHasTailCall() { HasTailCall = true; }
 
+  /// Computes the maximum size of a callframe and the AdjustsStack property.
+  /// This only works for targets defining
+  /// TargetInstrInfo::getCallFrameSetupOpcode(), getCallFrameDestroyOpcode(),
+  /// getFrameSize.
+  /// This is usually computed by the prologue epilogue inserter but some
+  /// targets may call this to compute it earlier.
+  void computeMaxCallFrameSize(const MachineFunction &MF);
+
   /// Return the maximum size of a call frame that must be
   /// allocated for an outgoing function call.  This is only available if
   /// CallFrameSetup/Destroy pseudo instructions are used by the target, and
   /// then only during or after prolog/epilog code insertion.
   ///
   unsigned getMaxCallFrameSize() const {
     // TODO: Enable this assert when targets are fixed.
     //assert(isMaxCallFrameSizeComputed() && "MaxCallFrameSize not computed yet");

lib/CodeGen/MachineFrameInfo.cpp

 #include "llvm/CodeGen/MachineFrameInfo.h"
 
 #include "llvm/ADT/BitVector.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetFrameLowering.h"
+#include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cassert>
 
 #define DEBUG_TYPE "codegen"
 
 using namespace llvm;
 
unsigned MachineFrameInfo::estimateStackSize(const MachineFunction &MF) const {
   // SP not FP. Align to MaxAlign so this works.
   StackAlign = std::max(StackAlign, MaxAlign);
   unsigned AlignMask = StackAlign - 1;
   Offset = (Offset + AlignMask) & ~uint64_t(AlignMask);
 
   return (unsigned)Offset;
 }
 
+void MachineFrameInfo::computeMaxCallFrameSize(const MachineFunction &MF) {
+  const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
+  unsigned FrameSetupOpcode = TII.getCallFrameSetupOpcode();
+  unsigned FrameDestroyOpcode = TII.getCallFrameDestroyOpcode();
+  assert(FrameSetupOpcode != ~0u && FrameDestroyOpcode != ~0u &&
+         "Can only compute MaxCallFrameSize if Setup/Destroy opcode are known");
+
+  MaxCallFrameSize = 0;
+  for (const MachineBasicBlock &MBB : MF) {
+    for (const MachineInstr &MI : MBB) {
+      unsigned Opcode = MI.getOpcode();
+      if (Opcode == FrameSetupOpcode || Opcode == FrameDestroyOpcode) {
+        unsigned Size = TII.getFrameSize(MI);
+        MaxCallFrameSize = std::max(MaxCallFrameSize, Size);
+        AdjustsStack = true;
+      } else if (MI.isInlineAsm()) {
+        // Some inline asm's need a stack frame, as indicated by operand 1.
+        unsigned ExtraInfo = MI.getOperand(InlineAsm::MIOp_ExtraInfo).getImm();
+        if (ExtraInfo & InlineAsm::Extra_IsAlignStack)
+          AdjustsStack = true;
+      }
+    }
+  }
+}
+
 void MachineFrameInfo::print(const MachineFunction &MF, raw_ostream &OS) const{
   if (Objects.empty()) return;
 
   const TargetFrameLowering *FI = MF.getSubtarget().getFrameLowering();
   int ValOffset = (FI ? FI->getOffsetOfLocalArea() : 0);
 
   OS << "Frame Objects:\n";
 

lib/CodeGen/PrologEpilogInserter.cpp

for (MachineBasicBlock::iterator I = BB->begin(); I != BB->end(); ++I)
         FrameSDOps.push_back(I);
       } else if (I->isInlineAsm()) {
         // Some inline asm's need a stack frame, as indicated by operand 1.
         unsigned ExtraInfo = I->getOperand(InlineAsm::MIOp_ExtraInfo).getImm();
         if (ExtraInfo & InlineAsm::Extra_IsAlignStack)
           AdjustsStack = true;
       }
 
+  assert(!MFI.isMaxCallFrameSizeComputed() ||
+         (MFI.getMaxCallFrameSize() == MaxCallFrameSize &&
+          MFI.adjustsStack() == AdjustsStack));
   MFI.setAdjustsStack(AdjustsStack);
   MFI.setMaxCallFrameSize(MaxCallFrameSize);
 
   for (std::vector<MachineBasicBlock::iterator>::iterator
          i = FrameSDOps.begin(), e = FrameSDOps.end(); i != e; ++i) {
     MachineBasicBlock::iterator I = *i;
 
     // If call frames are not being included as part of the stack frame, and

lib/Target/ARM/ARMBaseRegisterInfo.cpp

 ARMBaseRegisterInfo::getRegPressureLimit(const TargetRegisterClass *RC,
                                          MachineFunction &MF) const {
   const ARMSubtarget &STI = MF.getSubtarget<ARMSubtarget>();
   const ARMFrameLowering *TFI = getFrameLowering(MF);
 
   switch (RC->getID()) {
   default:
     return 0;
-  case ARM::tGPRRegClassID:
-    return TFI->hasFP(MF) ? 4 : 5;
+  case ARM::tGPRRegClassID: {
+    // hasFP ends up calling getMaxCallFrameComputed() which may not be
+    // available when getPressureLimit() is called as part of
+    // ScheduleDAGRRList.
+    bool HasFP = MF.getFrameInfo().isMaxCallFrameSizeComputed()
+                 ? TFI->hasFP(MF) : false;
+    return 5 - HasFP;
+  }
   case ARM::GPRRegClassID: {
-    unsigned FP = TFI->hasFP(MF) ? 1 : 0;
-    return 10 - FP - (STI.isR9Reserved() ? 1 : 0);
+    bool HasFP = MF.getFrameInfo().isMaxCallFrameSizeComputed()
+                 ? TFI->hasFP(MF) : false;
+    return 10 - HasFP - (STI.isR9Reserved() ? 1 : 0);
   }
   case ARM::SPRRegClassID:  // Currently not used as 'rep' register class.
   case ARM::DPRRegClassID:
     return 32 - 10;
   }
 }
 
 // Get the other register in a GPRPair.

lib/Target/ARM/ARMISelLowering.h

public:
     bool isLegalInterleavedAccessType(VectorType *VecTy,
                                       const DataLayout &DL) const;
 
     /// Returns the number of interleaved accesses that will be generated when
     /// lowering accesses of the given type.
     unsigned getNumInterleavedAccesses(VectorType *VecTy,
                                        const DataLayout &DL) const;
 
+    void finalizeLowering(MachineFunction &MF) const override;
+
   protected:
     std::pair<const TargetRegisterClass *, uint8_t>
     findRepresentativeClass(const TargetRegisterInfo *TRI,
                             MVT VT) const override;
 
   private:
     /// Subtarget - Keep a pointer to the ARMSubtarget around so that we can
     /// make the right decision when generating code for different targets.

lib/Target/ARM/ARMISelLowering.cpp

for (const MCPhysReg *I = IStart; *I; ++I) {
 
     // Insert the copy-back instructions right before the terminator.
     for (auto *Exit : Exits)
       BuildMI(*Exit, Exit->getFirstTerminator(), DebugLoc(),
               TII->get(TargetOpcode::COPY), *I)
           .addReg(NewVR);
   }
 }
+
+void ARMTargetLowering::finalizeLowering(MachineFunction &MF) const {
+  MF.getFrameInfo().computeMaxCallFrameSize(MF);
+  TargetLoweringBase::finalizeLowering(MF);
+}

test/CodeGen/ARM/alloca-align.ll

+; RUN: llc -o - %s | FileCheck %s
+target triple="arm--"
+
+@glob = external global i32*
+
+declare void @bar(i32*, [20000 x i8]* byval)
+
+; CHECK-LABEL: foo:
+; We should see the stack getting additional alignment
+; CHECK: sub sp, sp, #16
+; CHECK: bic sp, sp, #31
+; And a base pointer getting used.
+; CHECK: mov r6, sp
+; Which is passed to the call
+; CHECK: add [[REG:r[0-9]+]], r6, #19456
+; CHECK: add r0, [[REG]], #536
+; CHECK: bl bar
+define void @foo([20000 x i8]* %addr) {
+  %tmp = alloca [4 x i32], align 32
+  %tmp0 = getelementptr [4 x i32], [4 x i32]* %tmp, i32 0, i32 0
+  call void @bar(i32* %tmp0, [20000 x i8]* byval %addr)
+  ret void
+}
+