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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:511
+    // benefits generally outweight the cost, making the whole pipeline
+    // faster. See PR34652.
+    PM.add(createGlobalOptimizerPass());
----------------
davide wrote:
> mehdi_amini wrote:
> > chandlerc wrote:
> > > mehdi_amini wrote:
> > > > Did you run any benchmarks to conclude that it is faster? (The referenced PR seems to only mention some specific IR generated by Rust IIUC)
> > > > 
> > > I chatted some with Davide about this on IRC and he had seen similar issues with C++. Fundamentally, I would expect the same pattern to be able to come up in C++.
> > OK, it'd be nice to have a test-case with this patch though.
> Testing this feature is ... a little hard with the current infrastructure.
> I could write an -O2 test, if you'd like, but it's unclear how robust it is (in particular, if something before changes).
I've always felt we should have more of integration test (i.e. checking that code is, for instance, correctly vectorized when run through O2, etc.). Not as a lame replacement for targeted `opt` tests, but because end-to-end seems important.
I think Chandler told me that his view was that it should be caught by benchmarking, in which case we should observe an effect on the compile-time LNT suite for this change (or it would indicate possibly a missing test-case)



https://reviews.llvm.org/D38154