[PATCH] D38154: [PassManager] Run global opts after the inliner

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

I'm generally fine with this going in given the measurements.

Mehdi, would you feel comfortable with this as-is? I think even if we get more integration-style testing it will be hard to see this particular change because it should only materialize in compile time savings, and we may just not have any useful IR test cases in the test suite that would see that.

I think the closest thing to a way to test this change would be to take some pattern that is *really* bad without this and include it in the test suite (much like we have other IR test cases in the test suite from other frontends). But I'm not sure it makes sense to block this on landing that. I generally think that is something we should encourage the Rust community to work on...



================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:528
+  // benefits generally outweight the cost, making the whole pipeline
+  // faster. See PR34652.
+  if (RunInliner) {
----------------
We shouldn't include PR links in the source like this. Whatever context is needed should be summarized (and I think it already is).


https://reviews.llvm.org/D38154