[PATCH] D124193: [AMDGPU] Add WWM reserved VGPRs to WWMSpills
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 28 13:17:38 PDT 2022
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
More information about the llvm-commits
mailing list