[NFC] Combine runNewPMPasses() and runNewPMCustomPasses()

[aeubanks]

Summary

I've already witnessed two separate changes missing runNewPMPasses()
because runNewPMCustomPasses() is so similar.

This cleans up some duplicated code.

[aeubanks]

formatting

[tejohnson]

Looks like a great cleanup! One question below.

[aeubanks]

In D96553#2558668, @tejohnson wrote:

Looks like a great cleanup! One question below.

I don't see any comments

[tejohnson]

In D96553#2558797, @aeubanks wrote:

In D96553#2558668, @tejohnson wrote:

Looks like a great cleanup! One question below.

I don't see any comments

Weird, I guess I neglected to save it. Added it below.

llvm/lib/LTO/LTOBackend.cpp

211

Will setting the PGOOpt and passing down to the PassBuilder cause an issue for the custom case?

[aeubanks]

llvm/lib/LTO/LTOBackend.cpp

211

AFAICT the custom passes is mostly for experimenting with new custom pass pipelines, not the default pipelines that PassBuilder provides. PGOOpt is only used for PassBuilder's default pipelines.
Also there aren't a lot of tests that use it, and I think that the custom pipeline is not really used much in practice.

I guess what I'm saying is I'm not sure if this is really used and I'm don't think that it'll matter a lot.

[tejohnson]

lgtm

llvm/lib/LTO/LTOBackend.cpp

211

Ok. I'm going to assume that if one of these PGO types is enabled in the Config, that it should be honored. And if different behavior is desired, someone should put in a test for it.

[danilaml]

Don't know why no buildbot has caught it, but I think (haven't bisected yet, but this seems like the most likely culprit) this broke

LLVM :: tools/llvm-lto2/X86/pipeline.ll
lld :: ELF/lto/ltopasses-custom.ll

It seems that now, in both of these cases
; CHECK: define void @foo() {

has changed to
define void @foo() local_unnamed_addr #0 {
which is probably a good thing but tests need adjusting.

[bkramer]

In D96553#2563093, @danilaml wrote:

Don't know why no buildbot has caught it, but I think (haven't bisected yet, but this seems like the most likely culprit) this broke

LLVM :: tools/llvm-lto2/X86/pipeline.ll
lld :: ELF/lto/ltopasses-custom.ll

715dc556b782f718ce1815aaf5c58626f5fe839c should help

[danilaml]

In D96553#2564017, @bkramer wrote:

In D96553#2563093, @danilaml wrote:

Don't know why no buildbot has caught it, but I think (haven't bisected yet, but this seems like the most likely culprit) this broke

LLVM :: tools/llvm-lto2/X86/pipeline.ll
lld :: ELF/lto/ltopasses-custom.ll

715dc556b782f718ce1815aaf5c58626f5fe839c should help

Thanks, that fixed the tests.

[aeubanks]

Sorry about the breakages, I forgot that some people still have the legacy PM on by default.

llvm/lib/LTO/LTOBackend.cpp

createTargetMachine(const Config &Conf, const Target *TheTarget, Module &M) {
   assert(TM && "Failed to create target machine");
   return TM;
 }
 
 static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
                            unsigned OptLevel, bool IsThinLTO,
                            ModuleSummaryIndex *ExportSummary,
                            const ModuleSummaryIndex *ImportSummary) {
   Optional<PGOOptions> PGOOpt;
   if (!Conf.SampleProfile.empty())
     PGOOpt = PGOOptions(Conf.SampleProfile, "", Conf.ProfileRemapping,
                         PGOOptions::SampleUse, PGOOptions::NoCSAction, true);
   else if (Conf.RunCSIRInstr) {
     PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping,
                         PGOOptions::IRUse, PGOOptions::CSIRInstr);
   } else if (!Conf.CSIRProfile.empty()) {
     PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping,
                         PGOOptions::IRUse, PGOOptions::CSIRUse);
   }
 
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI(Conf.DebugPassManager);
   SI.registerCallbacks(PIC);
   PassBuilder PB(Conf.DebugPassManager, TM, Conf.PTO, PGOOpt, &PIC);
-  AAManager AA;
-
-  // Parse a custom AA pipeline if asked to.
-  if (auto Err = PB.parseAAPipeline(AA, "default"))
-    report_fatal_error("Error parsing default AA pipeline");
 
   RegisterPassPlugins(Conf.PassPlugins, PB);
 
   LoopAnalysisManager LAM(Conf.DebugPassManager);
   FunctionAnalysisManager FAM(Conf.DebugPassManager);
   CGSCCAnalysisManager CGAM(Conf.DebugPassManager);
   ModuleAnalysisManager MAM(Conf.DebugPassManager);
 
   std::unique_ptr<TargetLibraryInfoImpl> TLII(
       new TargetLibraryInfoImpl(Triple(TM->getTargetTriple())));
   if (Conf.Freestanding)
     TLII->disableAllFunctions();
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
 
+  AAManager AA;
+  // Parse a custom AA pipeline if asked to.
+  if (!Conf.AAPipeline.empty()) {
+    if (auto Err = PB.parseAAPipeline(AA, Conf.AAPipeline)) {
+      report_fatal_error("unable to parse AA pipeline description '" +
+                         Conf.AAPipeline + "': " + toString(std::move(Err)));
+    }
+  } else {
+    AA = PB.buildDefaultAAPipeline();
+  }
   // 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);
static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
   case 2:
     OL = PassBuilder::OptimizationLevel::O2;
     break;
   case 3:
     OL = PassBuilder::OptimizationLevel::O3;
     break;
   }
 
-  if (IsThinLTO)
+  // Parse a custom pipeline if asked to.
+  if (!Conf.OptPipeline.empty()) {
+    if (auto Err = PB.parsePassPipeline(MPM, Conf.OptPipeline)) {
+      report_fatal_error("unable to parse pass pipeline description '" +
+                         Conf.OptPipeline + "': " + toString(std::move(Err)));
+    }
+  } else if (IsThinLTO) {
     MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary));
-  else
+  } else {
     MPM.addPass(PB.buildLTODefaultPipeline(OL, ExportSummary));
-
-  if (!Conf.DisableVerify)
-    MPM.addPass(VerifierPass());
-
-  MPM.run(Mod, MAM);
   }
 
-static void runNewPMCustomPasses(const Config &Conf, Module &Mod,
-                                 TargetMachine *TM) {
-  PassBuilder PB(Conf.DebugPassManager, TM);
-  AAManager AA;
-
-  // Parse a custom AA pipeline if asked to.
-  if (!Conf.AAPipeline.empty())
-    if (auto Err = PB.parseAAPipeline(AA, Conf.AAPipeline))
-      report_fatal_error("unable to parse AA pipeline description '" +
-                         Conf.AAPipeline + "': " + toString(std::move(Err)));
-
-  RegisterPassPlugins(Conf.PassPlugins, PB);
-
-  LoopAnalysisManager LAM;
-  FunctionAnalysisManager FAM;
-  CGSCCAnalysisManager CGAM;
-  ModuleAnalysisManager MAM;
-
-  std::unique_ptr<TargetLibraryInfoImpl> TLII(
-      new TargetLibraryInfoImpl(Triple(TM->getTargetTriple())));
-  if (Conf.Freestanding)
-    TLII->disableAllFunctions();
-  FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
-
-  // 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;
-
-  // Always verify the input.
-  MPM.addPass(VerifierPass());
-
-  // Now, add all the passes we've been requested to.
-  if (auto Err = PB.parsePassPipeline(MPM, Conf.OptPipeline))
-    report_fatal_error("unable to parse pass pipeline description '" +
-                       Conf.OptPipeline + "': " + toString(std::move(Err)));
-
   if (!Conf.DisableVerify)
     MPM.addPass(VerifierPass());
+
   MPM.run(Mod, MAM);
 }
 
 static void runOldPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
                            bool IsThinLTO, ModuleSummaryIndex *ExportSummary,
                            const ModuleSummaryIndex *ImportSummary) {
   legacy::PassManager passes;
   passes.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
if (CmdArgs.empty())
       LLVM_DEBUG(
           dbgs() << "Post-(Thin)LTO merge bitcode embedding was requested, but "
                     "command line arguments are not available");
     llvm::EmbedBitcodeInModule(Mod, llvm::MemoryBufferRef(),
                                /*EmbedBitcode*/ true, /*EmbedCmdline*/ true,
                                /*Cmdline*/ CmdArgs);
   }
   // FIXME: Plumb the combined index into the new pass manager.
-  if (!Conf.OptPipeline.empty())
-    runNewPMCustomPasses(Conf, Mod, TM);
-  else if (Conf.UseNewPM)
+  if (Conf.UseNewPM) {
     runNewPMPasses(Conf, Mod, TM, Conf.OptLevel, IsThinLTO, ExportSummary,
                    ImportSummary);
-  else
+  } else {
     runOldPMPasses(Conf, Mod, TM, IsThinLTO, ExportSummary, ImportSummary);
+  }
   return !Conf.PostOptModuleHook || Conf.PostOptModuleHook(Task, Mod);
 }
 
 static void codegen(const Config &Conf, TargetMachine *TM,
                     AddStreamFn AddStream, unsigned Task, Module &Mod,
                     const ModuleSummaryIndex &CombinedIndex) {
   if (Conf.PreCodeGenModuleHook && !Conf.PreCodeGenModuleHook(Task, Mod))
     return;