[PATCH] D82451: [AMDGPU] Fix DPP Combiner:

[Valery Pykhtin via Phabricator via llvm-commits]

vpykhtin marked 3 inline comments as done.
vpykhtin added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:358
+// Constrain the number of found Uses by it's preallocated size
+template <unsigned N>
+bool collectIndependSubRegUse(MachineRegisterInfo &MRI, RegSubRegPair RSR,
----------------
arsenm wrote:
> Usually for an output small vector, you pass SmallVectorImpl<Type>& rather than needing a template parameter for the size
This is the way to pass the constraint on Uses. Initially the constraint was performed by execMayBeModifiedBeforeAnyUse but it's a bit late there. The constraint was introduced to reduce compile time so its of the same nature as a preallocation for Uses array and has similar value, so I decied to join them. Actually I don't need SmallVector here, but I haven't found vector with fixed size.


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:404-410
+  MachineInstr *PrevMI = nullptr;
+  for (auto &Use : Uses) {
+    auto *MI = Use->getParent();
+    if (MI == PrevMI)
+      return MI;
+    PrevMI = MI;
+  }
----------------
arsenm wrote:
> Why does this need to be a separate function? Can't you determine this over the initial use walk in collectRegUse?
There're two places where Uses are collected: collectRegUse and collectIndependSubRegUse - I would need to add this code twice. Number of uses is limited to be small and the walk is cheap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82451