[IPRA][ARM] Disable no-CSR optimisation for ARM

[ostannard]

Summary

This optimisation isn't generally profitable for ARM, because we can
save/restore many registers in the prologue and epilogue using the PUSH
and POP instructions, but mostly use individual LDR/STR instructions for
other spills.

[lebedev.ri]

llvm/include/llvm/CodeGen/TargetFrameLowering.h

367–369 ↗

(On Diff #210521)

Why not just add the enableNoCSROpt() hook instead?
The existing isSafeForNoCSROpt() seems well-defined..

[ostannard]

Split into separate functions for safe/profitable.

[ostannard]

Ping

[ostannard]

Ping.

[dmgreen]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

814 ↗

(On Diff #210544)

This is no longer needed?

[dmgreen]

Nice. LGTM

llvm/trunk/include/llvm/CodeGen/TargetFrameLowering.h

static bool isSafeForNoCSROpt(const Function &F) {
     // Function should not be optimized as tail call.
     for (const User *U : F.users())
       if (auto CS = ImmutableCallSite(U))
         if (CS.isTailCall())
           return false;
     return true;
   }
 
+  /// Check if the no-CSR optimisation is profitable for the given function.
+  virtual bool isProfitableForNoCSROpt(const Function &F) const {
+    return true;
+  }
+
   /// Return initial CFA offset value i.e. the one valid at the beginning of the
   /// function (before any stack operations).
   virtual int getInitialCFAOffset(const MachineFunction &MF) const;
 
   /// Return initial CFA register value i.e. the one valid at the beginning of
   /// the function (before any stack operations).
   virtual unsigned getInitialCFARegister(const MachineFunction &MF) const;
 };
 
 } // End llvm namespace
 
 #endif

llvm/trunk/lib/CodeGen/RegUsageInfoCollector.cpp

for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg) {
     }
     // If a register is in the UsedPhysRegsMask set then mark it as defined.
     // All clobbered aliases will also be in the set, so we can skip setting
     // as defined all the aliases here.
     if (UsedPhysRegsMask.test(PReg))
       SetRegAsDefined(PReg);
   }
 
-  if (TargetFrameLowering::isSafeForNoCSROpt(F)) {
+  if (TargetFrameLowering::isSafeForNoCSROpt(F) &&
+      MF.getSubtarget().getFrameLowering()->isProfitableForNoCSROpt(F)) {
     ++NumCSROpt;
     LLVM_DEBUG(dbgs() << MF.getName()
                       << " function optimized for not having CSR.\n");
   }
 
   LLVM_DEBUG(
     for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg) {
       if (MachineOperand::clobbersPhysReg(&(RegMask[0]), PReg))

llvm/trunk/lib/CodeGen/TargetFrameLoweringImpl.cpp

void TargetFrameLowering::determineCalleeSaves(MachineFunction &MF,
 
   // Resize before the early returns. Some backends expect that
   // SavedRegs.size() == TRI.getNumRegs() after this call even if there are no
   // saved registers.
   SavedRegs.resize(TRI.getNumRegs());
 
   // When interprocedural register allocation is enabled caller saved registers
   // are preferred over callee saved registers.
-  if (MF.getTarget().Options.EnableIPRA && isSafeForNoCSROpt(MF.getFunction()))
+  if (MF.getTarget().Options.EnableIPRA &&
+      isSafeForNoCSROpt(MF.getFunction()) &&
+      isProfitableForNoCSROpt(MF.getFunction()))
     return;
 
   // Get the callee saved register list...
   const MCPhysReg *CSRegs = MF.getRegInfo().getCalleeSavedRegs();
 
   // Early exit if there are no callee saved registers.
   if (!CSRegs || CSRegs[0] == 0)
     return;

 int TargetFrameLowering::getInitialCFAOffset(const MachineFunction &MF) const {
   llvm_unreachable("getInitialCFAOffset() not implemented!");
 }
 
 unsigned TargetFrameLowering::getInitialCFARegister(const MachineFunction &MF)
     const {
   llvm_unreachable("getInitialCFARegister() not implemented!");
 }
 No newline at end of file

llvm/trunk/lib/Target/ARM/ARMFrameLowering.h

public:
 
   void adjustForSegmentedStacks(MachineFunction &MF,
                                 MachineBasicBlock &MBB) const override;
 
   /// Returns true if the target will correctly handle shrink wrapping.
   bool enableShrinkWrapping(const MachineFunction &MF) const override {
     return true;
   }
+  bool isProfitableForNoCSROpt(const Function &F) const override {
+    // The no-CSR optimisation is bad for code size on ARM, because we can save
+    // many registers with a single PUSH/POP pair.
+    return false;
+  }
 
 private:
   void emitPushInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
                     const std::vector<CalleeSavedInfo> &CSI, unsigned StmOpc,
                     unsigned StrOpc, bool NoGap,
                     bool(*Func)(unsigned, bool), unsigned NumAlignedDPRCS2Regs,
                     unsigned MIFlags = 0) const;
   void emitPopInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,

llvm/trunk/test/CodeGen/ARM/ipra-no-csr.ll

+; RUN: llc -mtriple armv7a--none-eabi < %s              | FileCheck %s
+; RUN: llc -mtriple armv7a--none-eabi < %s -enable-ipra | FileCheck %s
+
+; Other targets disable callee-saved registers for internal functions when
+; using IPRA, but that isn't profitable for ARM because the PUSH/POP
+; instructions can more efficiently save registers than using individual
+; LDR/STRs in the caller.
+
+define internal void @callee() norecurse {
+; CHECK-LABEL: callee:
+entry:
+; CHECK: push {r4, lr}
+; CHECK: pop {r4, pc}
+  tail call void asm sideeffect "", "~{r4}"()
+  ret void
+}
+
+define void @caller() {
+entry:
+  call void @callee()
+  ret void
+}