[PATCH] D124193: [AMDGPU] Add WWM reserved VGPRs to WWMSpills

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
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));
+  }
----------------
arsenm wrote:
> 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
With the rename it's clearer what's going on here now. I'd prefer to keep the frame index creation in processFunctionBeforeFrameFinalized


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124193