[PATCH] D143762: [AMDGPU] Enable whole wave register copy

Fri Jun 23 07:58:06 PDT 2023

arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SILowerPredicatedCopies.cpp:159
+        LLVM_DEBUG(dbgs() << "Updated " << MI << " to use COPY opcode");
+        MI.setDesc(TII->get(AMDGPU::COPY));
+        Changed |= true;
----------------
yassingh wrote:
> yassingh wrote:
> > arsenm wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > yassingh wrote:
> > > > > > arsenm wrote:
> > > > > > > Do these need to gain an implicit exec use?
> > > > > > Do you mean we should add the implicit exec? In that case, SIFixVGPRCopies will take care?
> > > > > This isn't something that can be taken care of later. SIFixVGPRCopies is a horribly broken hack, the less we depend on it the better
> > > > Actually these split copies might have been the original problem which caused it to be added. Maybe we have a way to drop it now?
> > > That was the reasoning given in D28874 when it was added. How about as a next step we make sure all the VGPR splits end up with exec reads
> > Will look into it, don't have sufficient expertise to answer this right now. Adding @AMDGPU. 
> I can try and see what comes. I can see 2 ways to go about it, add the exec operand here itself or, in tablegen definition of PRED_COPY.  What do you suggest?
Add to the instruction definition. Though we're still stuck with COPY=PRED_COPY 'sometimes' 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143762