Address comments

Author: paperchalice

llvm/include/llvm/CodeGen/CodeGenPassBuilder.h

@@ -187,9 +187,9 @@ template <typename DerivedT> class CodeGenPassBuilder {
                     "Only module pass and function pass are supported.");
 
       // Add Function Pass
-      if constexpr (is_detected<is_function_pass_t, PassT>::value)
+      if constexpr (is_detected<is_function_pass_t, PassT>::value) {
         FPM.addPass(std::forward<PassT>(Pass));
-      else {
+      } else {
         // Add Module Pass
         if (!FPM.isEmpty()) {
           MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));

llvm/lib/Passes/PassBuilder.cpp

@@ -300,109 +300,13 @@ cl::opt<bool> PrintPipelinePasses(
              "(best-effort only)."));
 } // namespace llvm
 
-namespace {
-
-// The following passes/analyses have custom names, otherwise their name will
-// include `(anonymous namespace)`. These are special since they are only for
-// testing purposes and don't live in a header file.
-
-/// No-op module pass which does nothing.
-struct NoOpModulePass : PassInfoMixin<NoOpModulePass> {
-  PreservedAnalyses run(Module &M, ModuleAnalysisManager &) {
-    return PreservedAnalyses::all();
-  }
-
-  static StringRef name() { return "NoOpModulePass"; }
-};
-
-/// No-op module analysis.
-class NoOpModuleAnalysis : public AnalysisInfoMixin<NoOpModuleAnalysis> {
-  friend AnalysisInfoMixin<NoOpModuleAnalysis>;
-  static AnalysisKey Key;
-
-public:
-  struct Result {};
-  Result run(Module &, ModuleAnalysisManager &) { return Result(); }
-  static StringRef name() { return "NoOpModuleAnalysis"; }
-};
-
-/// No-op CGSCC pass which does nothing.
-struct NoOpCGSCCPass : PassInfoMixin<NoOpCGSCCPass> {
-  PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &,
-                        LazyCallGraph &, CGSCCUpdateResult &UR) {
-    return PreservedAnalyses::all();
-  }
-  static StringRef name() { return "NoOpCGSCCPass"; }
-};
-
-/// No-op CGSCC analysis.
-class NoOpCGSCCAnalysis : public AnalysisInfoMixin<NoOpCGSCCAnalysis> {
-  friend AnalysisInfoMixin<NoOpCGSCCAnalysis>;
-  static AnalysisKey Key;
-
-public:
-  struct Result {};
-  Result run(LazyCallGraph::SCC &, CGSCCAnalysisManager &, LazyCallGraph &G) {
-    return Result();
-  }
-  static StringRef name() { return "NoOpCGSCCAnalysis"; }
-};
-
-/// No-op function pass which does nothing.
-struct NoOpFunctionPass : PassInfoMixin<NoOpFunctionPass> {
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &) {
-    return PreservedAnalyses::all();
-  }
-  static StringRef name() { return "NoOpFunctionPass"; }
-};
-
-/// No-op function analysis.
-class NoOpFunctionAnalysis : public AnalysisInfoMixin<NoOpFunctionAnalysis> {
-  friend AnalysisInfoMixin<NoOpFunctionAnalysis>;
-  static AnalysisKey Key;
-
-public:
-  struct Result {};
-  Result run(Function &, FunctionAnalysisManager &) { return Result(); }
-  static StringRef name() { return "NoOpFunctionAnalysis"; }
-};
-
-/// No-op loop nest pass which does nothing.
-struct NoOpLoopNestPass : PassInfoMixin<NoOpLoopNestPass> {
-  PreservedAnalyses run(LoopNest &L, LoopAnalysisManager &,
-                        LoopStandardAnalysisResults &, LPMUpdater &) {
-    return PreservedAnalyses::all();
-  }
-  static StringRef name() { return "NoOpLoopNestPass"; }
-};
-
-/// No-op loop pass which does nothing.
-struct NoOpLoopPass : PassInfoMixin<NoOpLoopPass> {
-  PreservedAnalyses run(Loop &L, LoopAnalysisManager &,
-                        LoopStandardAnalysisResults &, LPMUpdater &) {
-    return PreservedAnalyses::all();
-  }
-  static StringRef name() { return "NoOpLoopPass"; }
-};
-
-/// No-op loop analysis.
-class NoOpLoopAnalysis : public AnalysisInfoMixin<NoOpLoopAnalysis> {
-  friend AnalysisInfoMixin<NoOpLoopAnalysis>;
-  static AnalysisKey Key;
-
-public:
-  struct Result {};
-  Result run(Loop &, LoopAnalysisManager &, LoopStandardAnalysisResults &) {
-    return Result();
-  }
-  static StringRef name() { return "NoOpLoopAnalysis"; }
-};
-
 AnalysisKey NoOpModuleAnalysis::Key;
 AnalysisKey NoOpCGSCCAnalysis::Key;
 AnalysisKey NoOpFunctionAnalysis::Key;
 AnalysisKey NoOpLoopAnalysis::Key;
 
+namespace {
+
 /// Whether or not we should populate a PassInstrumentationCallbacks's class to
 /// pass name map.
 ///

llvm/unittests/CodeGen/CodeGenPassBuilderTest.cpp

@@ -23,21 +23,6 @@ using namespace llvm;
 
 namespace {
 
-struct NoOpModulePass : PassInfoMixin<NoOpModulePass> {
-  PreservedAnalyses run(Module &M, ModuleAnalysisManager &) {
-    return PreservedAnalyses::all();
-  }
-
-  static StringRef name() { return "NoOpModulePass"; }
-};
-
-struct NoOpFunctionPass : PassInfoMixin<NoOpFunctionPass> {
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &) {
-    return PreservedAnalyses::all();
-  }
-  static StringRef name() { return "NoOpFunctionPass"; }
-};
-
 class DummyCodeGenPassBuilder
     : public CodeGenPassBuilder<DummyCodeGenPassBuilder> {
 public:
@@ -67,6 +52,7 @@ class CodeGenPassBuilderTest : public testing::Test {
     InitializeAllTargets();
     InitializeAllTargetMCs();
 
+    // TODO: Move this test to normal lit test when llc supports new pm.
     static const char *argv[] = {
         "test",
         "-print-pipeline-passes",
@@ -85,6 +71,8 @@ class CodeGenPassBuilderTest : public testing::Test {
     TargetOptions Options;
     TM = static_cast<LLVMTargetMachine *>(
         TheTarget->createTargetMachine("", "", "", Options, std::nullopt));
+    if (!TM)
+      GTEST_SKIP();
   }
 };
 
@@ -118,16 +106,10 @@ TEST_F(CodeGenPassBuilderTest, basic) {
     return PassName.empty() ? Name : PassName;
   });
   const char ExpectedIRPipeline[] =
-      "require<profile-summary>,require<collector-metadata>,"
-      "PreISelIntrinsicLoweringPass,function(verify,loop-mssa(loop-reduce),"
-      "mergeicmps,expand-memcmp,gc-lowering),shadow-stack-gc-lowering,function("
-      "lower-constant-intrinsics,UnreachableBlockElimPass,consthoist,"
-      "ReplaceWithVeclib,partially-inline-libcalls,ee-instrument<post-inline>,"
-      "scalarize-masked-mem-intrin,ExpandReductionsPass,codegenprepare,"
-      "dwarf-eh-prepare),no-op-module,function(no-op-function,no-op-function,"
-      "no-op-function),no-op-module,function(no-op-function,callbrprepare,"
-      "safe-stack,stack-protector,verify)";
-  EXPECT_EQ(IRPipeline, ExpectedIRPipeline);
+      "no-op-module,function(no-op-function,"
+      "no-op-function,no-op-function),no-op-module";
+  // TODO: Move this test to normal lit test when llc supports new pm.
+  EXPECT_TRUE(StringRef(IRPipeline).contains(ExpectedIRPipeline));
 
   std::string MIRPipeline;
   raw_string_ostream MIROS(MIRPipeline);
@@ -151,8 +133,9 @@ TEST_F(CodeGenPassBuilderTest, basic) {
       "FEntryInserterPass,XRayInstrumentationPass,PatchableFunctionPass,"
       "FuncletLayoutPass,StackMapLivenessPass,LiveDebugValuesPass,"
       "MachineSanitizerBinaryMetadata,FreeMachineFunctionPass";
-  EXPECT_EQ(MIRPipeline, ExpectedMIRPipeline);
   // TODO: Check pipeline string when all pass names are populated.
+  // TODO: Move this test to normal lit test when llc supports new pm.
+  EXPECT_EQ(MIRPipeline, ExpectedMIRPipeline);
 }
 
 } // namespace