Honestly, I think this canonicalizer traversal change patch was rushed in based on a single test case (or two) and without adequate experimentation for something that has more far reaching consequences on transformation pipelines than anything else – and more importantly with the quick conclusion that there was no reason a bottom up traversal could be better. Pre-order or a different approach might well be a better one eventually, but I don’t think a major change like this that has impact on nearly every MLIR user upstream or downstream should be done without adequate experimentation. I feel the right course of action now is to quickly revert this. For eg., we can’t be sure that it doesn’t slow down a large number of cases by small amounts upstream and downstream or create other issues. We need a good representative set even if small and the pass timing infra already exists to manually time and check for that small number.
Yes, but the reason the traversal order was kept unchanged for this long was the realization that post order + processing in reverse was a natural one for “consumer-driven” simplification. I recall reading and updating things in the rewrite driver multiple times but not considering changing the traversal order for that reason.