[CodeGen][AArch64] Don't split functions with a red zone on AArch64

[dhoekwater]

Summary

Because unconditional branch relaxation on AArch64 grows the stack to
spill a register, splitting a function would cause the red zone to be
overwritten. Explicitly disable MFS for such functions.

[leonardchan]

Sorry I accidentally closed the revision. I think just re-opening it should be fine then when you commit it, you can still use this differential revision.

[leonardchan]

I think perhaps you can re-upload your diff to the original one.

[dhoekwater]

In D157127#4562390, @leonardchan wrote:

I think perhaps you can re-upload your diff to the original one.

All good, that worked.

[dhoekwater]

Use an LLVM IR test instead of an MIR test since those are easier to read. Removes dependency on https://reviews.llvm.org/D157063.

[snehasish]

It would be great to reuse the existing corpus of splitting tests for ARM. They would cover basic functionality along with interactions with different flavours of FDO. Can you create a separate patch to move the existing tests in test/CodeGen/X86/machine-function-splitter.ll to test/CodeGen/Generic/machine-function-splitter.ll? Then we can add the triple and appropriate CHECKs for x86 and ARM.

One precedent I can find is: test/CodeGen/Generic/cfi-sections.ll which has x86 and ARM triples (and REQUIRES the x86 and ARM registered targets).

[dhoekwater]

Rebase onto D157565. Remove a few reviewers since I added too many initially.

[dhoekwater]

Add tests into Generic

[dhoekwater]

Rebase onto main and remove the dependency

[dhoekwater]

Update the revision description

llvm/include/llvm/CodeGen/TargetInstrInfo.h

llvm_unreachable("Target didn't implement "
                      "TargetInstrInfo::isFunctionSafeToOutlineFrom!");
   }
 
   /// Return true if the function should be outlined from by default.
   virtual bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const {
     return false;
   }
 
+  /// Return true if the function is a viable candidate for machine function
+  /// splitting. The criteria for if a function can be split may vary by target.
+  virtual bool isFunctionSafeToSplit(const MachineFunction &MF) const;
+
   /// Produce the expression describing the \p MI loading a value into
   /// the physical register \p Reg. This hook should only be used with
   /// \p MIs belonging to VReg-less functions.
   virtual std::optional<ParamLoadedValue>
   describeLoadedValue(const MachineInstr &MI, Register Reg) const;
 
   /// Given the generic extension instruction \p ExtMI, returns true if this
   /// extension is a likely candidate for being folded into an another

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/CodeGen/BasicBlockSectionUtils.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineBlockFrequencyInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/Passes.h"
+#include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/CommandLine.h"
 #include <optional>
 
 using namespace llvm;
 
 // FIXME: This cutoff value is CPU dependent and should be moved to

 bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
   // We target functions with profile data. Static information in the form
   // of exception handling code may be split to cold if user passes the
   // mfs-split-ehcode flag.
   bool UseProfileData = MF.getFunction().hasProfileData();
   if (!UseProfileData && !SplitAllEHCode)
     return false;
 
-  // TODO: We don't split functions where a section attribute has been set
-  // since the split part may not be placed in a contiguous region. It may also
-  // be more beneficial to augment the linker to ensure contiguous layout of
-  // split functions within the same section as specified by the attribute.
-  if (MF.getFunction().hasSection() ||
-      MF.getFunction().hasFnAttribute("implicit-section-name"))
+  const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
+  if (!TII.isFunctionSafeToSplit(MF))
     return false;
 
-  // We don't want to proceed further for cold functions
-  // or functions of unknown hotness. Lukewarm functions have no prefix.
-  std::optional<StringRef> SectionPrefix = MF.getFunction().getSectionPrefix();
-  if (SectionPrefix &&
-      (*SectionPrefix == "unlikely" || *SectionPrefix == "unknown")) {
-    return false;
-  }
-
   // Renumbering blocks here preserves the order of the blocks as
   // sortBasicBlocksAndUpdateBranches uses the numeric identifier to sort
   // blocks. Preserving the order of blocks is essential to retaining decisions
   // made by prior passes such as MachineBlockPlacement.
   MF.RenumberBlocks();
   MF.setBBSectionsType(BasicBlockSection::Preset);
 
   MachineBlockFrequencyInfo *MBFI = nullptr;

llvm/lib/CodeGen/TargetInstrInfo.cpp

bool TargetInstrInfo::hasLowDefLatency(const TargetSchedModel &SchedModel,
   if (!ItinData || ItinData->isEmpty())
     return false;
 
   unsigned DefClass = DefMI.getDesc().getSchedClass();
   int DefCycle = ItinData->getOperandCycle(DefClass, DefIdx);
   return (DefCycle != -1 && DefCycle <= 1);
 }
 
+bool TargetInstrInfo::isFunctionSafeToSplit(const MachineFunction &MF) const {
+  // TODO: We don't split functions where a section attribute has been set
+  // since the split part may not be placed in a contiguous region. It may also
+  // be more beneficial to augment the linker to ensure contiguous layout of
+  // split functions within the same section as specified by the attribute.
+  if (MF.getFunction().hasSection() ||
+      MF.getFunction().hasFnAttribute("implicit-section-name"))
+    return false;
+
+  // We don't want to proceed further for cold functions
+  // or functions of unknown hotness. Lukewarm functions have no prefix.
+  std::optional<StringRef> SectionPrefix = MF.getFunction().getSectionPrefix();
+  if (SectionPrefix &&
+      (*SectionPrefix == "unlikely" || *SectionPrefix == "unknown")) {
+    return false;
+  }
+
+  return true;
+}
+
 std::optional<ParamLoadedValue>
 TargetInstrInfo::describeLoadedValue(const MachineInstr &MI,
                                      Register Reg) const {
   const MachineFunction *MF = MI.getMF();
   const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
   DIExpression *Expr = DIExpression::get(MF->getFunction().getContext(), {});
   int64_t Offset;
   bool OffsetIsScalable;

llvm/lib/Target/AArch64/AArch64InstrInfo.h

public:
   static bool isFalkorShiftExtFast(const MachineInstr &MI);
   /// Return true if the instructions is a SEH instruciton used for unwinding
   /// on Windows.
   static bool isSEHInstruction(const MachineInstr &MI);
 
   std::optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
                                            Register Reg) const override;
 
+  bool isFunctionSafeToSplit(const MachineFunction &MF) const override;
+
   std::optional<ParamLoadedValue>
   describeLoadedValue(const MachineInstr &MI, Register Reg) const override;
 
   unsigned int getTailDuplicateSize(CodeGenOpt::Level OptLevel) const override;
 
   bool isExtendLikelyToBeFolded(MachineInstr &ExtMI,
                                 MachineRegisterInfo &MRI) const override;
 

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

describeORRLoadedValue(const MachineInstr &MI, Register DescribedReg,
   }
 
   assert(!TRI->isSuperOrSubRegisterEq(DestReg, DescribedReg) &&
          "Unhandled ORR[XW]rs copy case");
 
   return std::nullopt;
 }
 
+bool AArch64InstrInfo::isFunctionSafeToSplit(const MachineFunction &MF) const {
+  // Functions cannot be split to different sections on AArch64 if they have
+  // a red zone. This is because relaxing a cross-section branch may require
+  // incrementing the stack pointer to spill a register, which would overwrite
+  // the red zone.
+  if (MF.getInfo<AArch64FunctionInfo>()->hasRedZone().value_or(true))
+    return false;
+
+  return TargetInstrInfo::isFunctionSafeToSplit(MF);
+}
+
 std::optional<ParamLoadedValue>
 AArch64InstrInfo::describeLoadedValue(const MachineInstr &MI,
                                       Register Reg) const {
   const MachineFunction *MF = MI.getMF();
   const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
   switch (MI.getOpcode()) {
   case AArch64::MOVZWi:
   case AArch64::MOVZXi: {

llvm/test/CodeGen/AArch64/machine-function-splitter.mir

--- |
   4:                                                ; preds = %1
     %5 = call i32 @baz()
     br label %6
 
   6:                                                ; preds = %4, %2
     %7 = tail call i32 @qux()
     ret void
   }
-  
+  ; Function Attrs: nounwind
+  define void @nosplit_redzone(i1 zeroext %0) #0 !prof !14 !section_prefix !15 {
+    br i1 %0, label %2, label %4, !prof !16
+
+  2:                                                ; preds = %1
+    %3 = call i32 @bar()
+    br label %6
+
+  4:                                                ; preds = %1
+    %5 = call i32 @baz()
+    br label %6
+
+  6:                                                ; preds = %4, %2
+    %7 = tail call i32 @qux()
+    ret void
+  }
+
   declare i32 @bar()
   
   declare i32 @baz()
   
   declare i32 @bam()
   
   declare i32 @qux()
   
bb.1 (%ir-block.2):
     TCRETURNdi @qux, 0, csr_aarch64_aapcs, implicit $sp
 
   bb.2 (%ir-block.4):
     BL @baz, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $w0
     early-clobber $sp, $lr = frame-destroy LDRXpost $sp, 16 :: (load (s64) from %stack.0)
     TCRETURNdi @qux, 0, csr_aarch64_aapcs, implicit $sp
 
 ...
+---
+name:            nosplit_redzone
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16,
+      stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+machineFunctionInfo:
+  hasRedZone:      true
+body:             |
+  ; CHECK-LABEL: name: nosplit_redzone
+  ; COM: Check that cold blocks in functions with red zones aren't split.
+  ; CHECK: bb.0 (%ir-block.1)
+  ; CHECK-NOT: bb.1 (%ir-block.2, bbsections Cold)
+  ; CHECK-NOT: bb.2 (%ir-block.4, bbsections Cold)
+
+  bb.0 (%ir-block.1):
+    successors: %bb.1(0x80000000), %bb.2(0x00000000)
+    liveins: $w0, $lr
+
+    early-clobber $sp = frame-setup STRXpre killed $lr, $sp, -16 :: (store (s64) into %stack.0)
+    CBZW killed renamable $w0, %bb.2
+
+  bb.1 (%ir-block.2):
+    BL @bar, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $w0
+    early-clobber $sp, $lr = frame-destroy LDRXpost $sp, 16 :: (load (s64) from %stack.0)
+    TCRETURNdi @qux, 0, csr_aarch64_aapcs, implicit $sp
+
+  bb.2 (%ir-block.4):
+    BL @baz, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def dead $w0
+    early-clobber $sp, $lr = frame-destroy LDRXpost $sp, 16 :: (load (s64) from %stack.0)
+    TCRETURNdi @qux, 0, csr_aarch64_aapcs, implicit $sp
+
+...