[PATCH] D29627: Compile time decreasing in the case we're dealing with Machine Combiner

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM.

Please put a perf measurement in the commit message (or add a test to test-suite so we don't regress?) - something like:

Before this patch (testing on N GHz Haswell):
$ time llc -o /dev/null -enable-unsafe-fp-math foo.ll 
X secs
After this patch:
$ time llc -o /dev/null -enable-unsafe-fp-math foo.ll 
Y secs 
foo.ll is attached to https://reviews.llvm.org/D29627



================
Comment at: lib/CodeGen/MachineCombiner.cpp:449-450
       } else {
+        // We're getting the trace when we really need it for computations
+        MachineTraceMetrics::Trace BlockTrace = MinInstr->getTrace(MBB);
+        if (improvesCriticalPathLen(MBB, &MI, BlockTrace, InsInstrs, DelInstrs,
----------------
How about making the comment more direct, so we don't lose track of the reason. Something like:
"Calculating the trace metrics may be expensive, so only do this when necessary."


https://reviews.llvm.org/D29627