[PATCH] D76884: [AMDGPU] Implement -amdgpu-spill-cfi-saved-regs

[David Greene via Phabricator via llvm-commits]

greened added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:148
 
-  int64_t Offset = MFI.getObjectOffset(FI);
+  int64_t Offset = MFI.getObjectOffset(FI) + DwordOff;
 
----------------
Add a comment about why this is here.  What is DwordOff about?


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:672
 
+void SIFrameLowering::emitPrologueEntryCFI(MachineBasicBlock &MBB,
+                                           MachineBasicBlock::iterator MBBI,
----------------
Needs a comment explaining what this does.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:772
+
+void SIFrameLowering::emitCFISavedRegSpills(MachineFunction &MF,
+                                            MachineBasicBlock &MBB,
----------------
Needs a comment explaining what this does.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1356
         (I != FuncInfo->FramePointerSaveIndex &&
-         I != FuncInfo->BasePointerSaveIndex)) {
+         I != FuncInfo->BasePointerSaveIndex &&
+         (!TRI->isCFISavedRegsSpillEnabled() ||
----------------
Can this be simplified by splitting into multiple if statements and commented?  All of the chained logical operations are difficult to parse.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1408
 
+static void allocateCFISave(MachineFunction &MF, int &FI, Register Reg) {
+  SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
----------------
Needs a comment.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1636
 
+bool SIFrameLowering::spillCalleeSavedRegisters(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
----------------
Needs a comment.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1724
 
+void SIFrameLowering::buildCFIForSGPRToVGPRSpill(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
----------------
Needs a comment.


================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:428
 
-void SIMachineFunctionInfo::removeDeadFrameIndices(MachineFrameInfo &MFI) {
-  // The FP & BP spills haven't been inserted yet, so keep them around.
+void SIMachineFunctionInfo::removeDeadFrameIndices(MachineFunction &MF) {
+  MachineFrameInfo &MFI = MF.getFrameInfo();
----------------
Needs a comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76884/new/

https://reviews.llvm.org/D76884