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

[krishna chaitanya sankisa via Phabricator via llvm-commits]

skc7 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:914
+      for (auto &U : MRI->use_instructions(Reg)) {
+        if (TRI->isSGPRReg(*MRI, Reg) && !TII->isVALU(*Inst))
           Users.push_back(&U);
----------------
alex-t wrote:
> What is the reason for swapping these lines? 
> if this
> ```
> TRI->isSGPRReg(*MRI, Reg) && !TII->isVALU(*Inst)
> ```
> is not true we don't need to process register users.
Fixed previous patch. Swap for and if was my mistake. Also added check for AMDGPU::SReg_32RegClass used by soffset of MUBUF/MTBUF.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:922
+            const MachineOperand *soffsetMO =
+                TII->getNamedOperand(U, AMDGPU::OpName::soffset);
+
----------------
alex-t wrote:
> skc7 wrote:
> > foad wrote:
> > > I don't understand why we need special cases for particular named operands. Why can't this be inferred from the operand's register class? @alex-t?
> > soffset and srsrc need to be scalar registers in mubuf/mtbuf instructions.. Check for use of COPY's result is done here for these operands.
> The fact that the specific use requires SGPR surely can be checked for any use.
> What I don't like here is the hack forcing the decision to be made for v_readfistlane.
> The algorithm that makes a decision is a complex tradeoff and we should not "tune" it in such a way for each particular case. I would consider the "SGPR required" as a weight value for the solver rather than the ultimate condition.
HasMBUFScalarReg flag would be set to true for scalar operands of MUBUF/MTBUF. This would lower copy to use v_readfirstlane_b32.


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

https://reviews.llvm.org/D134423