Fix the bug that parseAAPipeline is not invoked in runNewPMPasses in release compiler.

[danielcdh]

Summary

The logic is guarded by "assert".

[davide]

this is a no brainer, but do you mind to add a test, if possible?

[chandlerc]

Doh! Agree with Davide about a test. I can try to conjure up one if it isn't easy...

[danielcdh]

Tried to add a test. But either need to have

1. a testcase that go through all default optimization pipeline, and make a difference with and without the default AA pipeline (tried test/Analysis/BasicAA/invariant_load.ll, but does not work)
2. introduce something like -debug-pass-manager to llvm-lto2 so that we can check for passes invoked by llvm-lto2

#2 is beyond the scope of this patch. So I plan to commit the patch as is. I could work on #2 later (maybe after I get AutoFDO + newPM performance back to normal).

Please let me know if the plan does not sound OK, otherwise I'll commit patch before I leave work today.

Thanks,
Dehao

[davide]

Wondering whether that should be a report_fatal_error instead of llvm_unreachable? (as I did in runNewPMCustomPasses).
[apologies if I'm missing something, I have this code out of cache and my head deep in GVN today].

Also, either 1) or 2) is fine. I'd prefer 2), and I'd really prefer this to be now. If it's too trouble, maybe open a PR and add a FIXME here.
I understand this is blocking a lot of people, so, if Chandler is OK, this can go in as is (with my previous comments addressed).

BTW, thanks for catching this braino!

[danielcdh]

Add test.

[danielcdh]

I've added the test. Please let me know if you want me to split the patch in 2.

Thanks,
Dehao

[davide]

LGTM. No need to split.

[chandlerc]

LGTM as well FWIW, but happy to defer to Davide here.

[davide]

Go for it, hopefully you can commit this today and unblock people :)

include/llvm/LTO/Config.h

struct Config {
   std::string SampleProfile;
 
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
   /// Whether to emit optimization remarks with hotness informations.
   bool RemarksWithHotness = false;
 
+  /// Whether to emit the pass manager debuggging informations.
+  bool DebugPassManager = false;
+
   bool ShouldDiscardValueNames = true;
   DiagnosticHandlerFunction DiagHandler;
 
   /// If this field is set, LTO will write input file paths and symbol
   /// resolutions here in llvm-lto2 command line flag format. This can be
   /// used for testing and for running the LTO pipeline outside of the linker
   /// with llvm-lto2.
   std::unique_ptr<raw_ostream> ResolutionFile;

lib/LTO/LTOBackend.cpp

static void runNewPMPasses(Config &Conf, Module &Mod, TargetMachine *TM,
   Optional<PGOOptions> PGOOpt;
   if (!Conf.SampleProfile.empty())
     PGOOpt = PGOOptions("", "", Conf.SampleProfile, false, true);
 
   PassBuilder PB(TM, PGOOpt);
   AAManager AA;
 
   // Parse a custom AA pipeline if asked to.
-  assert(PB.parseAAPipeline(AA, "default"));
+  if (!PB.parseAAPipeline(AA, "default"))
+    report_fatal_error("Error parsing default AA pipeline");
 
-  LoopAnalysisManager LAM;
-  FunctionAnalysisManager FAM;
-  CGSCCAnalysisManager CGAM;
-  ModuleAnalysisManager MAM;
+  LoopAnalysisManager LAM(Conf.DebugPassManager);
+  FunctionAnalysisManager FAM(Conf.DebugPassManager);
+  CGSCCAnalysisManager CGAM(Conf.DebugPassManager);
+  ModuleAnalysisManager MAM(Conf.DebugPassManager);
 
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
 
   // Register all the basic analyses with the managers.
   PB.registerModuleAnalyses(MAM);
   PB.registerCGSCCAnalyses(CGAM);
   PB.registerFunctionAnalyses(FAM);
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
-  ModulePassManager MPM;
+  ModulePassManager MPM(Conf.DebugPassManager);
   // FIXME (davide): verify the input.
 
   PassBuilder::OptimizationLevel OL;
 
   switch (OptLevel) {
   default:
     llvm_unreachable("Invalid optimization level");
   case 0:
     OL = PassBuilder::O0;
     break;
   case 1:
     OL = PassBuilder::O1;
     break;
   case 2:
     OL = PassBuilder::O2;
     break;
   case 3:
     OL = PassBuilder::O3;
     break;
   }
 
   if (IsThinLTO)
-    MPM = PB.buildThinLTODefaultPipeline(OL, false /* DebugLogging */);
+    MPM = PB.buildThinLTODefaultPipeline(OL, Conf.DebugPassManager);
   else
-    MPM = PB.buildLTODefaultPipeline(OL, false /* DebugLogging */);
+    MPM = PB.buildLTODefaultPipeline(OL, Conf.DebugPassManager);
   MPM.run(Mod, MAM);
 
   // FIXME (davide): verify the output.
 }
 
 static void runNewPMCustomPasses(Module &Mod, TargetMachine *TM,
                                  std::string PipelineDesc,
                                  std::string AAPipelineDesc,

test/tools/llvm-lto2/X86/pipeline.ll

 ; RUN: llvm-as < %s > %t1.bc
 
+; Try the default pipeline and check is BasicAA is invoked.
+; RUN: llvm-lto2 run %t1.bc -o %t.o -r %t1.bc,patatino,px -debug-pass-manager \
+; RUN:  -use-new-pm 2>&1 | FileCheck %s --check-prefix=DEFAULT
+; DEFAULT: Running analysis: BasicAA on patatino
+
 ; Try a custom pipeline
 ; RUN: llvm-lto2 run %t1.bc -o %t.o -save-temps \
 ; RUN:  -r %t1.bc,patatino,px -opt-pipeline loweratomic \
 ; RUN:  -aa-pipeline basic-aa
 ; RUN: llvm-dis < %t.o.0.4.opt.bc | FileCheck %s --check-prefix=CUSTOM
 
 ; Try the new pass manager LTO default pipeline (make sure the option
 ; is accepted).

tools/llvm-lto2/llvm-lto2.cpp

static cl::opt<std::string>
     SamplePGOFile("lto-sample-profile-file",
                   cl::desc("Specify a SamplePGO profile file"));
 
 static cl::opt<bool>
     UseNewPM("use-new-pm",
              cl::desc("Run LTO passes using the new pass manager"),
              cl::init(false), cl::Hidden);
 
+static cl::opt<bool>
+    DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
+                     cl::desc("Print pass management debugging information"));
+
 static void check(Error E, std::string Msg) {
   if (!E)
     return;
   handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
     errs() << "llvm-lto2: " << Msg << ": " << EIB.message().c_str() << '\n';
   });
   exit(1);
 }
static int run(int argc, char **argv) {
 
   Conf.CPU = MCPU;
   Conf.Options = InitTargetOptionsFromCodeGenFlags();
   Conf.MAttrs = MAttrs;
   if (auto RM = getRelocModel())
     Conf.RelocModel = *RM;
   Conf.CodeModel = CMModel;
 
+  Conf.DebugPassManager = DebugPassManager;
+
   if (SaveTemps)
     check(Conf.addSaveTemps(OutputFilename + "."),
           "Config::addSaveTemps failed");
 
   // Optimization remarks.
   Conf.RemarksFilename = OptRemarksOutput;
   Conf.RemarksWithHotness = OptRemarksWithHotness;