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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.
Herald added subscribers: kosarev, jsilvanus.


================
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));
----------------
cdevadas wrote:
> cdevadas wrote:
> > arsenm wrote:
> > > 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?
> > The separate list was maintained for Serialize info. We could directly add them into LaneVGPRs otherwise.
> The best place to fill in the LaneVGPRs (both writelane and WWM reserved VGPRs) would be in `processFunctionBeforeFrameFinalized`. But that is invoked after `determineCalleeSaves` where we mask these spills off the CSR list so that the default spill insertion in PEI would defer them, and we could later custom insert them during FrameLowering.
> 
> At this moment I don't see a better place to move it. We could add a condition to enable this code only once if `determineCalleeSaves` needs to be called multiple times.
The serialization is supposed to reflect whatever's here. It's not worth preserving for its own sake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124193