[PATCH] D42838: [AMDGPU] added writelane intrinsic

[Tim Renouf via Phabricator via llvm-commits]

tpr added inline comments.


================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.cpp:748
       BuildMI(*MBB, MI, DL,
-              TII->getMCOpcodeFromPseudo(AMDGPU::V_WRITELANE_B32),
+              TII->get(AMDGPU::V_WRITELANE_B32),
               Spill.VGPR)
----------------
nhaehnle wrote:
> tpr wrote:
> > arsenm wrote:
> > > dstuttard wrote:
> > > > I notice that you've left this as the pseudo whereas the original lowered to an actual MCOpcode - I guess it's more consistent to leave that final lowering until later.
> > > There is a reason for this, but I don't remember what it was exactly. I think there was some related change between SI and VI. I think you should leave this as a separate change. I know I've tried to fix this before and hit some issue.
> > Yes; gfx6 and gfx7 have vop2 and gfx8 has vop3. But I saw that my new use of writelane does not lower the pseudo until later, and my testing with this change (with llpc only, not other users of the backend) passed. But good idea to make it a separate change.  
> What is actually breaking without the RegState::Undef?
> 
> When we allocate a VGPR for spilling, we add it as LiveIn to all basic blocks, and the V_READLANE_B32 generated for restoring a spilled SGPR should never set the VGPR to killed. So I don't see where the machine verifier would complain. An example would be good.
Just re-checked, and I think the problem (shown by test/CodeGen/AMDGPU/byval-frame-setup.ll) is when it happens to pick a callee saves register for spilling sgprs into. After inserting the writelanes for sgpr spills, it runs the prologue/epilogue insertion pass, which adds a store of the callee saves register to stack frame, with a kill.

Any ideas on a good way to fix that? Anything less hacky than scanning the first basic block to find the first writelane and mark just that one as undef?


Repository:
  rL LLVM

https://reviews.llvm.org/D42838