[PATCH] D88291: [AMDGPU] Insert waterfall loops for divergent calls

Tue Sep 29 03:10:54 PDT 2020

madhur13490 added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:5127
+    MachineOperand *Dest = &MI.getOperand(0);
+    if (!RI.isSGPRClass(MRI.getRegClass(Dest->getReg()))) {
+      // Also move the copies to physical registers into the loop block
----------------
Flakebi wrote:
> madhur13490 wrote:
> > Should this block be executed for AGPRs too? If this is meant only for VGPRs then !SGPR is not correct.
> I don’t know how AGPRs work and documentation seems to be scarce, the check for calls is the same as for image operations above.
> It seems like AGPRs can be copied to VGPRs but not to SGPRs (https://reviews.llvm.org/D68358#change-cnuV0NuhUhzL), so I think calling a function pointer that is stored in AGPRs should copy them to VGPRs and insert a waterfall loop.
My main concern is that this check is over relaxing and allows all non-SGPR classes. This also includes any future register class we may add. AGPR is just an example. What about enabling it just for VGPR for now and put a TODO?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88291