[PATCH] D36619: [MachineCombiner] Update instruction depths incrementally for large BBs.

[Gerolf Hoflehner via Phabricator via llvm-commits]

Gerolf added a comment.

LGTM.

I added a few minor remarks that you might want to consider before commit. Again, thanks for working on this and your patience!

Cheers
Gerolf



================
Comment at: lib/CodeGen/MachineCombiner.cpp:396
+  if (IncrementalUpdate)
+    for (auto *InstrPtr : InsInstrs)
+      MinInstr->updateDepth(MBB, *InstrPtr, RegUnits);
----------------
You could call updateDepths here as well. Also if you had code like if (IncrementalUpdate) MinInstr->updateDepths(...) else MinInstr->invalidate(MBB) you could spare two parameters (at the cost of adding such a line after each call). It feels more natural to me than packing the code inside the insertDelete...() procedure.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:417
   auto BlockIter = MBB->begin();
+  auto LastUpdate = MBB->begin();
   // Check if the block is in a loop.
----------------
LastUpdate = BlockIter like in the code below?


https://reviews.llvm.org/D36619