[PATCH] D95117: [NewPM][opt] Run the "default" AA pipeline by default

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/tools/opt/NewPMDriver.cpp:364
   if (!Passes.empty() && !DisableBasicAA) {
     if (auto Err = PB.parseAAPipeline(AA, "basic-aa")) {
       errs() << Arg0 << ": " << toString(std::move(Err)) << "\n";
----------------
bjope wrote:
> Isn't the default in the legacy PM "default" rather than "basic-aa"?
> 
> I think that by using "default" here the backwards compatibility would be more correct (and the example given by @markus above would yield same result as without `-enable-new-pm`). I'd be happy to create a patch if that sounds reasonable.
The legacy PM defaults to only having basic-aa by default, see `AAResultsWrapperPass::runOnFunction()`. Something like `-O2` will add other alias analyses to the pipeline, see `PassManagerBuilder::addInitialAliasAnalysisPasses()` and other places where `PassManagerBuilder` will add extra AA analyses.

I don't see a reason to upgrade all tests that are using the `opt -instcombine` syntax to run the default set of alias analyses rather than just basic-aa. I'd rather not change tests while changing the value of `-enable-new-pm` since this affects almost every test. Incrementally fixing up tests to only use `opt -passes=instcombine` will change the AA pipeline that they use (if the test doesn't manually specify its own), but I think that's ok.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95117/new/

https://reviews.llvm.org/D95117