[PATCH] D41766: [MachineCombiner] Add check for optimal pattern order.

[Gerolf Hoflehner via Phabricator via llvm-commits]

Gerolf added a comment.

I like the spirit of the idea. What made you look into this? Some more questions and suggestions below, but LGTM as is.

Cheers
Gerolf



================
Comment at: lib/CodeGen/MachineCombiner.cpp:485
 
+#ifdef EXPENSIVE_CHECKS
+    // Check that the difference between original and new latency is
----------------
Could that go into a separate function? You might want to control it by an internal flag rather than a compile-time define.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:506
+      long CurrentLatencyDiff = ((long)RootLatency) - ((long)NewRootLatency);
+      assert(CurrentLatencyDiff <= PrevLatencyDiff &&
+             "Current pattern is better than previous pattern.");
----------------
You might want to add a dumping capability that prints the pattern in a sorted order. Did you encounter scenarios where different orderings of pattern (other than the default) gave improvements in some cases, but not others? If these cases can be tight to some characteristic in the program it might be possible to pass this information on to getMachineCombinerPatterns() and return a different order.


https://reviews.llvm.org/D41766