[PM] Simplify how we parse the outer layer of the pass pipeline text and

remove an extra, redundant pass manager wrapping every run.

I had kept seeing these when manually testing, but it was getting really
annoying and was going to cause problems with overly eager invalidation.
The root cause was an overly complex and unnecessary pile of code for
parsing the outer layer of the pass pipeline. We can instead delegate
most of this to the recursive pipeline parsing.

I've added some somewhat more basic and precise tests to catch this.

llvm-svn: 225253

llvm/test/Other/new-pass-manager.ll

@@ -5,6 +5,42 @@
 ; files, but for now this is just going to step the new process through its
 ; paces.
 
+; RUN: opt -disable-output -disable-verify -debug-pass-manager -debug-cgscc-pass-manager \
+; RUN:     -passes=no-op-module %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-MODULE-PASS
+; CHECK-MODULE-PASS: Starting module pass manager
+; CHECK-MODULE-PASS-NEXT: Running module pass: NoOpModulePass
+; CHECK-MODULE-PASS-NEXT: Finished module pass manager run.
+
+; RUN: opt -disable-output -disable-verify -debug-pass-manager -debug-cgscc-pass-manager \
+; RUN:     -passes=no-op-cgscc %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-CGSCC-PASS
+; RUN: opt -disable-output -disable-verify -debug-pass-manager -debug-cgscc-pass-manager \
+; RUN:     -passes='cgscc(no-op-cgscc)' %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-CGSCC-PASS
+; CHECK-CGSCC-PASS: Starting module pass manager
+; CHECK-CGSCC-PASS-NEXT: Running module pass: ModuleToPostOrderCGSCCPassAdaptor
+; CHECK-CGSCC-PASS-NEXT: Running module analysis: CGSCCAnalysisManagerModuleProxy
+; CHECK-CGSCC-PASS-NEXT: Running module analysis: Lazy CallGraph Analysis
+; CHECK-CGSCC-PASS-NEXT: Starting CGSCC pass manager run.
+; CHECK-CGSCC-PASS-NEXT: Running CGSCC pass: NoOpCGSCCPass
+; CHECK-CGSCC-PASS-NEXT: Finished CGSCC pass manager run.
+; CHECK-CGSCC-PASS-NEXT: Finished module pass manager run.
+
+; RUN: opt -disable-output -disable-verify -debug-pass-manager -debug-cgscc-pass-manager \
+; RUN:     -passes=no-op-function %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-FUNCTION-PASS
+; RUN: opt -disable-output -disable-verify -debug-pass-manager -debug-cgscc-pass-manager \
+; RUN:     -passes='function(no-op-function)' %s 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-FUNCTION-PASS
+; CHECK-FUNCTION-PASS: Starting module pass manager
+; CHECK-FUNCTION-PASS-NEXT: Running module pass: ModuleToFunctionPassAdaptor
+; CHECK-FUNCTION-PASS-NEXT: Running module analysis: FunctionAnalysisManagerModuleProxy
+; CHECK-FUNCTION-PASS-NEXT: Starting function pass manager run.
+; CHECK-FUNCTION-PASS-NEXT: Running function pass: NoOpFunctionPass
+; CHECK-FUNCTION-PASS-NEXT: Finished function pass manager run.
+; CHECK-FUNCTION-PASS-NEXT: Finished module pass manager run.
+
 ; RUN: opt -disable-output -debug-pass-manager -passes=print %s 2>&1 \
 ; RUN:     | FileCheck %s --check-prefix=CHECK-MODULE-PRINT
 ; CHECK-MODULE-PRINT: Starting module pass manager

llvm/test/Other/pass-pipeline-parsing.ll

@@ -34,12 +34,9 @@
 ; CHECK-NESTED-TWO-NOOP-FP: Starting module pass manager
 ; CHECK-NESTED-TWO-NOOP-FP: Running module pass: ModuleToFunctionPassAdaptor
 ; CHECK-NESTED-TWO-NOOP-FP: Starting function pass manager
-; CHECK-NESTED-TWO-NOOP-FP: Running function pass: FunctionPassManager
-; CHECK-NESTED-TWO-NOOP-FP: Starting function pass manager
 ; CHECK-NESTED-TWO-NOOP-FP: Running function pass: NoOpFunctionPass
 ; CHECK-NESTED-TWO-NOOP-FP: Running function pass: NoOpFunctionPass
 ; CHECK-NESTED-TWO-NOOP-FP: Finished function pass manager
-; CHECK-NESTED-TWO-NOOP-FP: Finished function pass manager
 ; CHECK-NESTED-TWO-NOOP-FP: Finished module pass manager
 
 ; RUN: opt -disable-output -debug-pass-manager \

llvm/tools/opt/Passes.cpp

@@ -347,34 +347,21 @@ static bool parseModulePassPipeline(ModulePassManager &MPM,
 // pre-populate the analysis managers with target-specific stuff?
 bool llvm::parsePassPipeline(ModulePassManager &MPM, StringRef PipelineText,
                              bool VerifyEachPass) {
-  // Look at the first entry to figure out which layer to start parsing at.
+  // By default, try to parse the pipeline as-if it were within an implicit
-  if (PipelineText.startswith("module("))
+  // 'module(...)' pass pipeline. If this will parse at all, it needs to
-    return parseModulePassPipeline(MPM, PipelineText, VerifyEachPass) &&
+  // consume the entire string.
-           PipelineText.empty();
+  if (parseModulePassPipeline(MPM, PipelineText, VerifyEachPass))
-  if (PipelineText.startswith("cgscc(")) {
+    return PipelineText.empty();
-    CGSCCPassManager CGPM;
-    if (!parseCGSCCPassPipeline(CGPM, PipelineText, VerifyEachPass) ||
-        !PipelineText.empty())
-      return false;
-    MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
-    return true;
-  }
-  if (PipelineText.startswith("function(")) {
-    FunctionPassManager FPM;
-    if (!parseFunctionPassPipeline(FPM, PipelineText, VerifyEachPass) ||
-        !PipelineText.empty())
-      return false;
-    MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
-    return true;
-  }
 
-  // This isn't a direct pass manager name, look for the end of a pass name.
+  // This isn't parsable as a module pipeline, look for the end of a pass name
+  // and directly drop down to that layer.
   StringRef FirstName =
       PipelineText.substr(0, PipelineText.find_first_of(",)"));
-  if (isModulePassName(FirstName))
+  assert(!isModulePassName(FirstName) &&
-    return parseModulePassPipeline(MPM, PipelineText, VerifyEachPass) &&
+         "Already handled all module pipeline options.");
-           PipelineText.empty();
 
+  // If this looks like a CGSCC pass, parse the whole thing as a CGSCC
+  // pipeline.
   if (isCGSCCPassName(FirstName)) {
     CGSCCPassManager CGPM;
     if (!parseCGSCCPassPipeline(CGPM, PipelineText, VerifyEachPass) ||
@@ -384,6 +371,8 @@ bool llvm::parsePassPipeline(ModulePassManager &MPM, StringRef PipelineText,
     return true;
   }
 
+  // Similarly, if this looks like a Function pass, parse the whole thing as
+  // a Function pipelien.
   if (isFunctionPassName(FirstName)) {
     FunctionPassManager FPM;
     if (!parseFunctionPassPipeline(FPM, PipelineText, VerifyEachPass) ||