[PATCH] D71089: [AMDGPU] Optimizing unnecessary copies for REG_SEQUENCE PHI operand. Also fixes rocBLAS error

Fri Dec 6 09:27:25 PST 2019

arsenm added a comment.

Definitely needs a test. I'm not sure I understand the fix here. This is some kind of workaround? I would be happier with the subreg fix happening first. Removing unnecessary copies should not be relevant to the correctness fix?

================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:275
-
-  MRI.setRegClass(DstReg, DstRC);
-
----------------
Factoring this out is a separate patch

================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4274

 void SIInstrInfo::legalizeGenericOperand(MachineBasicBlock &InsertMBB,
                                          MachineBasicBlock::iterator I,
----------------
I think the legalization done here should only touch the local instruction, and not be scanning through copy chains. This seems like it's trying to avoid a failure of the PeepholeOptimizer?

================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4290-4294
+  MachineInstr *Def = MRI.getVRegDef(OpReg);
+  if (Op.getParent()->isPHI() && !RI.isSGPRClass(DstRC) &&
+      RI.isSGPRClass(OpRC)) {
+    SmallVector<MachineInstr*, 4> CopiesToDelete;
+    while (Def && Def->isCopy() && MRI.hasOneUse(Def->getOperand(0).getReg())) {
----------------
Needs comments

================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4303
+          OpReg, DstRC);
+        for (auto& Copy : CopiesToDelete)
+          Copy->eraseFromParent();
----------------
Doesn't need to be a reference

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

https://reviews.llvm.org/D71089