[PATCH] D64393: [AMDGPU] Fix DPP combiner check for exec modification

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6103-6106
+  auto E = UseMI->getIterator();
+  for (auto I = std::next(DefMI.getIterator()); I != E; ++I) {
+    if (I->isDebugInstr())
+      continue;
----------------
foad wrote:
> arsenm wrote:
> > This should probably be a backwards scan, but that can be a separate change
> What's the advantage?
I was thinking about kills, but it doesn't matter in SSA


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6144-6145
 
-  // Stop scan at the use if known.
-  auto E = UseMI ? UseMI->getIterator() : DefBB->end();
-  for (auto I = std::next(DefMI.getIterator()); I != E; ++I) {
+  // Stop scan when we have seen all the uses.
+  for (auto I = std::next(DefMI.getIterator()); !Uses.empty(); ++I) {
     if (I->isDebugInstr())
----------------
foad wrote:
> arsenm wrote:
> > Why is this erasing the use as it iterates?
> So we can exit the loop when Uses.empty().
I mean, this usage of a set doesn't make sense to me. Why can't you just increment a seen use count?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64393