[PATCH] D133301: [ipra] Fix missing save/restore of function return addresses

[Colin McEwan via Phabricator via llvm-commits]

cme created this revision.
cme added reviewers: mehdi_amini, arsenm.
Herald added subscribers: atanasyan, jrtc27, hiraditya, arichardson, sdardis.
Herald added a project: All.
cme requested review of this revision.
Herald added subscribers: llvm-commits, wdng.
Herald added a project: LLVM.

     

This patch adds the architecture-neutral components of a fix for issue #57482 ( https://github.com/llvm/llvm-project/issues/57482 ), and the artitecture-specific parts of the fix for MIPS processors.

IPRA allows the save/restore sequences to be omitted from callees whose clobber sets have been fully accounted for by the calling function due to IPRA. However, since the return address to return from the callee is used in the *return* instruction of the calling instruction rather than in the caller, having the caller preserve a value is inadequate for return addresses.

No existing register property available in the framework encapsulates this "needed for return instruction" property, so this changed adds this as a TargerRegisterInfo query method.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133301

Files:
  llvm/include/llvm/CodeGen/TargetRegisterInfo.h
  llvm/lib/CodeGen/TargetFrameLoweringImpl.cpp
  llvm/lib/Target/Mips/MipsRegisterInfo.cpp
  llvm/lib/Target/Mips/MipsRegisterInfo.h


Index: llvm/lib/Target/Mips/MipsRegisterInfo.h
===================================================================
--- llvm/lib/Target/Mips/MipsRegisterInfo.h
+++ llvm/lib/Target/Mips/MipsRegisterInfo.h
@@ -72,6 +72,9 @@
   /// Return GPR register class.
   virtual const TargetRegisterClass *intRegClass(unsigned Size) const = 0;
 
+  virtual bool isNeededForReturn(MCRegister PhysReg,
+                                 const MachineFunction &MF) const override;
+
 private:
   virtual void eliminateFI(MachineBasicBlock::iterator II, unsigned OpNo,
                            int FrameIndex, uint64_t StackSize,
Index: llvm/lib/Target/Mips/MipsRegisterInfo.cpp
===================================================================
--- llvm/lib/Target/Mips/MipsRegisterInfo.cpp
+++ llvm/lib/Target/Mips/MipsRegisterInfo.cpp
@@ -244,6 +244,11 @@
   return true;
 }
 
+bool MipsRegisterInfo::isNeededForReturn(MCRegister PhysReg,
+                                         const MachineFunction &MF) const {
+  return PhysReg == Mips::RA || PhysReg == Mips::RA_64;
+}
+
 // FrameIndex represent objects inside a abstract stack.
 // We must replace FrameIndex with an stack/frame pointer
 // direct reference.
Index: llvm/lib/CodeGen/TargetFrameLoweringImpl.cpp
===================================================================
--- llvm/lib/CodeGen/TargetFrameLoweringImpl.cpp
+++ llvm/lib/CodeGen/TargetFrameLoweringImpl.cpp
@@ -92,10 +92,9 @@
 
   // When interprocedural register allocation is enabled caller saved registers
   // are preferred over callee saved registers.
-  if (MF.getTarget().Options.EnableIPRA &&
-      isSafeForNoCSROpt(MF.getFunction()) &&
-      isProfitableForNoCSROpt(MF.getFunction()))
-    return;
+  bool NoCSR = (MF.getTarget().Options.EnableIPRA &&
+                isSafeForNoCSROpt(MF.getFunction()) &&
+                isProfitableForNoCSROpt(MF.getFunction()));
 
   // Get the callee saved register list...
   const MCPhysReg *CSRegs = MF.getRegInfo().getCalleeSavedRegs();
@@ -124,10 +123,12 @@
   // Functions which call __builtin_unwind_init get all their registers saved.
   bool CallsUnwindInit = MF.callsUnwindInit();
   const MachineRegisterInfo &MRI = MF.getRegInfo();
+  const TargetRegisterInfo *RI = MF.getSubtarget().getRegisterInfo();
   for (unsigned i = 0; CSRegs[i]; ++i) {
     unsigned Reg = CSRegs[i];
     if (CallsUnwindInit || MRI.isPhysRegModified(Reg))
-      SavedRegs.set(Reg);
+      if (!NoCSR || !MRI.isAllocatable(Reg) || RI->isNeededForReturn(Reg, MF))
+        SavedRegs.set(Reg);
   }
 }
 
Index: llvm/include/llvm/CodeGen/TargetRegisterInfo.h
===================================================================
--- llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -578,6 +578,14 @@
     return false;
   }
 
+  // Return true if the register is needed for returning from the
+  // function and so must be preserved in the callee even if preserved
+  // by the caller
+  virtual bool isNeededForReturn(MCRegister PhysReg,
+                                 const MachineFunction &MF) const {
+    return false;
+  }
+
   /// Prior to adding the live-out mask to a stackmap or patchpoint
   /// instruction, provide the target the opportunity to adjust it (mainly to
   /// remove pseudo-registers that should be ignored).


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133301.457967.patch
Type: text/x-patch
Size: 3344 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220905/4745082d/attachment.bin>