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

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle 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
----------------
madhur13490 wrote:
> 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?
> 
> 
`Dest` is the destination we're calling, i.e. the function pointer. The point of the logic, AFAIU, is that if the function pointer is non-uniform, we need to do something about that. AGPRs are non-uniform... so I'd say `!isSGPR` is correct. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88291