[PATCH] D53762: AMDGPU: Combine DPP mov with use instuctions (VOP1/2/3)

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/GCNDPPCombine.cpp:9-10
+//===----------------------------------------------------------------------===//
+// This pass combines dpp moves with the using instructions
+//===----------------------------------------------------------------------===//
+
----------------
The comments in your commit message would be more useful here


================
Comment at: lib/Target/AMDGPU/GCNDPPCombine.cpp:228-229
+  case AMDGPU::V_MAX_U32_e32:
+    if (OldOpndValue.getImm() == std::numeric_limits<unsigned>::max())
+      return OldOpndVGPR;
+    break;
----------------
Why do you need to handle these cases? I would also use uint32_t instead of unsigned


================
Comment at: lib/Target/AMDGPU/GCNDPPCombine.cpp:323-324
+  LLVM_DEBUG(dbgs() << "  old=";
+    if (!OldOpndValue) dbgs() << "undef";
+    else dbgs() << OldOpndValue->getImm();
+    dbgs() << ", bound_ctrl=" << BoundCtrlZero << '\n');
----------------
New lines


================
Comment at: lib/Target/AMDGPU/GCNDPPCombine.cpp:408
+
+  std::vector<MachineInstr*> DPPMoves;
+  for (auto &MBB : MF) {
----------------
Why do you need to collect a separate list of the moves in the whole function? Can you just use the dfs iterator to avoid this?


Repository:
  rL LLVM

https://reviews.llvm.org/D53762