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

[Alexander via Phabricator via llvm-commits]

alex-t 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);
----------------
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.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:917
+
+        if (Inst->isCopy()) {
+          unsigned Opc = U.getOpcode();
----------------
Inst not necessarily is a copy. You can have MUBUF/MTBUF instruction terminating the SALU chain of arbitrary length. The V2S copy result may be (for example) used by some long arithmetic sequence and then used as an operand of the REG_SEQUENCE which produces SReg_128, which in order used as an operand in MUBUF instruction.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:921
+          // need to be scalar registers.
+          if (TII->isMUBUF(Opc) || TII->isMTBUF(Opc)) {
+            const MachineOperand *soffsetMO =
----------------
Maybe just 

```
          if (MRI->getRegClass(Reg) == &AMDGPU::SReg_128RegClass &&
             (TII->isMUBUF(Opc) || TII->isMTBUF(Opc))
```
?


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

https://reviews.llvm.org/D134423