[PATCH] D134423: [AMDGPU] Fix vgpr2sgpr copy analysis to check scalar operands of buffer instructions use scalar registers.

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

>   define <4 x i32> @extract0_bitcast_raw_buffer_load_v4i32(<4 x i32> inreg %rsrc, i32 %ofs, i32 %sofs) local_unnamed_addr #0 {
>   %var = tail call <4 x i32> @llvm.amdgcn.raw.buffer.load.v4i32(<4 x i32> %rsrc, i32 %ofs, i32 %sofs, i32 0)
>   ret <4 x i32> %var
>   }
>
> IR dump after amdgpu-isel:
>
>   bb.0 (%ir-block.0):
>   liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4, $vgpr5



>   %5:vgpr_32 = COPY $vgpr5



>   %8:sreg_32 = COPY %5:vgpr_32



>   %7:vreg_128 = BUFFER_LOAD_DWORDX4_OFFEN %4:vgpr_32, killed %6:sgpr_128, %8:sreg_32, 0, 0, 0, 0, implicit $exec :: (dereferenceable load (s128), align 1, addrspace 4)

Arguments of non-kernel function are divergent. So, $vgpr5 is divergent and %5 as well.



================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:920
+               MRI->getRegClass(Reg) == &AMDGPU::SReg_32RegClass) &&
+              (TII->isMUBUF(Opc) || TII->isMTBUF(Opc))) {
+            Info.HasMBUFScalarReg = true;
----------------
arsenm wrote:
> The MUBUF/MTBUF part isn't interesting, it's the operand being an SGPR and not trivially rewritable as vector 
We are walking SALU def-use chain to compute the score to decide if it is profitable to insert v_readfirstlane_b32 or convert a copy and all the chain to VALU.
If we remove 
```
(TII->isMUBUF(Opc) || TII->isMTBUF(Opc))
```
we will end up prohibiting any VALU conversion. Because all the SALU result registers are SGPRs and many of them have SGPR_128RegClass and SReg_32RegClass. We don't want to cut all of them.

The idea was that the exact opcodes are exceptional because following the common logic leads to inserting the waterfall loop that slows down the execution.


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

https://reviews.llvm.org/D134423