[PATCH] D40330: Separate ExecutionDepsFix into 4 parts - enable breaking false dependencies for all reg classes.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: include/llvm/CodeGen/ExecutionDepsFix.h:136
+/// ReachingDefAnalysis and ExecutionDomainFix.
+/// It identifies basic blocks that are part of loops and should to be visited twise 
+/// and returns efficient traversal order for all the blocks.
----------------
twise->twice


================
Comment at: include/llvm/CodeGen/ExecutionDepsFix.h:170
+
+  const SmallVector<TraversedMBBInfo, 4> traverse(MachineFunction &MF);
+
----------------
Remove the const. It doesn't mean anything if you're returning a whole object.


================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:878
+  const SmallVectorImpl<LoopTraversal::TraversedMBBInfo>
+      &TraversedMBBInfoOrder = Traversal.traverse(mf);
+  for (auto TraversedMBB : TraversedMBBInfoOrder) {
----------------
traverse is returning an object not a reference so we shouldn't be capturing by reference.

You may want to consider having the caller pass a SmallVector to traverse. That way traverse doesn't have to hardcode an explicit size for the SmallVector.


================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:897
+}
+bool ReachingDefAnalysis::runOnMachineFunction(MachineFunction &mf) {
+  if (skipFunction(*mf.getFunction()))
----------------
Add blank line before this function


https://reviews.llvm.org/D40330