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

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 1 23:23:51 PDT 2017


davide added a subscriber: dberlin.
davide 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());
----------------
mehdi_amini wrote:
> 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)
> 
At some point @dberlin told me about the GCC infrastructure for end-to-end pipeline testing (and how it helped him to find a series of bugs in the past) for this so I ended up taking a look.
It allows to break at arbitrary points in the pipeline and make sure you actually do the right thing in the next pass, so it's fair more robust in case passes are introduced before/after the one you want to test.
In theory, `opt-bisect=X` kinda approximates this behaviour, but it's more an helper for debugging than an helper for testing. 

Maybe `lit` could grow a series of directive, e.g `--check-stop-before=PASSNAME` so that we check the IR right before and right after that pass has run on a given function. It of course gets a little more complicated when the unit of IR on which you're working on is not anymore a function, but, e.g. an SCC, but having the function pass pipeline better tested end-to-end would be already an enormous improvement to the status quo.


https://reviews.llvm.org/D38154





More information about the llvm-commits mailing list