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

[Gerolf Hoflehner via Phabricator via llvm-commits]

Gerolf added a comment.

Hi Florian

This looks promising! Thanks for working on this nagging problem spot. More out of curiosity: Could you also share the data for your compile-time improvements?

Cheers
Gerolf



================
Comment at: lib/CodeGen/MachineCombiner.cpp:280
     return NewRootDepth < RootDepth;
 
   unsigned NewRootLatency = getLatency(Root, NewRoot, BlockTrace);
----------------
What happened to the comment?


================
Comment at: lib/CodeGen/MachineCombiner.cpp:298
+               << NewCycleCount << "\n";
+        if (SlackIsAccurate)
+          dbgs() << " RootDepth + RootLatency + RootSlack = "
----------------
SlackIsAccurate is checked twice for OldCycleCount?! I think the old scheme is fine which just dumps the components of new and old root latencies. Perhaps only add whether or not SlackIsAccurate is set?


================
Comment at: lib/CodeGen/MachineCombiner.cpp:487
       } else {
-        // Calculating the trace metrics may be expensive,
-        // so only do this when necessary.
+        // For big basic blocks, Wwe only compute the full trace the first time
+        // we hit this the first  time. We do not invalidate the trace, but
----------------
Typos (eg. Wwe -> We) and dups ('the first time'). What does 'relatively accurate' mean?


https://reviews.llvm.org/D36619