[PATCH] D124192: [AMDGPU] Callee must always spill writelane VGPRs

[Matt Arsenault via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1263
+      if (MI.getOpcode() == AMDGPU::V_WRITELANE_B32)
+        MFI->addToLaneVGPRs(MF, MI.getOperand(0).getReg());
+    }
----------------
cdevadas wrote:
> arsenm wrote:
> > cdevadas wrote:
> > > arsenm wrote:
> > > > I don't think this is the right place to determine these registers. It's adding an extra loop over the function, and adding statefulness to determineCalleeSaves. Theoretically we should be able to call it multiple times 
> > > Any recommendation?
> > > This should be done before identifying the CSR registers and they should be skipped from the default CSR spill insertion routines.
> > Logically I think this belongs in PEI::calculateCallFrameInfo, but this isn't really a reasonable thing to add to the generic code.
> Yes, we can't accommodate the code into PEI::calculateCallFrameInfo.  The processFunctionBeforeFrameFinalized is also not a viable option. At this moment, determineCalleeSaves seemed a better place. The need to iterate over the MBB, in this case, can't be avoided.
I don't like it, but I guess we can go with this for now. Is there a way to assert that this was only computed once? I know at one point I had a patch that tried to speculatively call this to see what spills will be later inserted


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124192