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

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas 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:
> 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.


================
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));
+  }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124193