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

Wed Jul 13 09:48:38 PDT 2022

sebastian-ne added a comment.

In D124193#3618553 <https://reviews.llvm.org/D124193#3618553>, @cdevadas wrote:

> @sebastian-ne Is there a case when `SIPreAllocateWWMRegs` pass allocates a CSR VGPR to operands in WWM instructions?
> If it is true, we may have a bug in the current handling that we failed to reset the `WWMReservedRegs` from the `SavedVGPRs` during `determineCalleeSaves` that allow us later to custom insert these spills in the emitPrologue/emitEpilogue functions.
>
> Here is what we do for VGPRs used for SGPR spills.
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp#L1278
>
> It doesn't come under the scope of this patch. But if this case can arise, we must fix it.

Sorry for getting back to you so late. You are correct, `WWMReservedRegs` should be removed from the `SavedVGPRs` during `determineCalleeSaves`.

Currently, if WWM registers fall on callee-saves, they are spilled twice, once as CSR, once as WWM register. At least it’s not incorrect.
Here’s a test demonstrating this: https://github.com/Flakebi/amdvlk-llvm-project/commit/0b0e056c5be110096a92c8175d6650d8535d4e33#diff-eddae1d72701ad027181cd89a7409a0b653da8debae655c3379c5b4d97934a73R1241-R1246

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124193