[PATCH] D68828: [AMDGPU] Allow DPP combiner to work with REG_SEQUENCE

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 10 15:21:31 PDT 2019


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:420
   SmallVector<MachineInstr*, 4> OrigMIs, DPPMIs;
+  SmallSetVector<MachineInstr*, 4> RegSeqs;
   auto CombOldVGPR = getRegSubRegPair(*OldOpnd);
----------------
Why is this a SetVector?


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:459-462
+        if (OrigMI.getOperand(I).getReg() == DPPMovReg) {
+          FwdSubReg = OrigMI.getOperand(I + 1).getImm();
+          break;
+        }
----------------
I think this won't work in the case where the operand itself has a subregister. 
Can you add a test with something like

%0:vreg_64 = REG_SEQUENCE %vreg_64.sub0, sub1, %vreg_64.1, sub0

I think you can use composeSubRegIndices here


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:471
+      continue;
+    } else if (TII->isVOP3(OrigOp)) {
       if (!TII->hasVALU32BitEncoding(OrigOp)) {
----------------
No else after continue


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

https://reviews.llvm.org/D68828





More information about the llvm-commits mailing list