[PATCH] D124193: [AMDGPU] Add WWM reserved VGPRs to lane VGPRs list

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1247
+    const TargetRegisterClass *RC = TRI->getPhysRegClass(Reg);
+    MFI->addToLaneVGPRs(MF, Reg, TRI->getSpillSize(*RC),
+                        TRI->getSpillAlign(*RC));
----------------
arsenm wrote:
> Theoretically we could want to call determineCalleeSaves multiple times, so I'm not sure accumulating state here is a good idea.
If we're just going to add these to lane VGPRs, is there any real reason to distinguish the WWM registers? Could we just add them to LaneVGPRs to start with?


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1247-1248
+    const TargetRegisterClass *RC = TRI->getPhysRegClass(Reg);
+    MFI->addToLaneVGPRs(MF, Reg, TRI->getSpillSize(*RC),
+                        TRI->getSpillAlign(*RC));
+  }
----------------
Theoretically we could want to call determineCalleeSaves multiple times, so I'm not sure accumulating state here is a good idea.


================
Comment at: llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp:151
 
+    // To be spilled/restored in the prologue/epilogue.
     MFI->reserveWWMRegister(PhysReg);
----------------
Drop this comment, it's not unconditionally true. The relevant problem is addressed in frame lowering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124193