[NewPM] Support optnone under new pass manager

[aeubanks]

Summary

This uses pass instrumentation callbacks to skip optional passes.
PassInfoMixin now declares that passes inheriting from it are by default
optional. Using RequiredPassInfoMixin overrides the pass to be required.
The new OptNoneInstrumentation is part of StandardInstrumentations.

The feature of skipping optional passes for optnone functions under NPM
is gated on a -enable-npm-optnone flag. Currently it is by default
false. That is because we still need to mark all required passes to be
required. Otherwise optnone functions will start behaving incorrectly.
After that is done in following changes, we can remove the flag and
always enable this.

All adaptors/managers must be required, since the pass(es) they are
wrapping may be required.

In the future, opt-bisect will use this same mechanmism of determining
which passes are required/optional.

Depends on D83498.

[ychen]

llvm/include/llvm/IR/PassInstrumentation.h

150 ↗

(On Diff #276860)

Could we do this to not changing the callback API?
ShouldRun &= C(Pass.name(), llvm::Any(&IR)) || Pass.isRequired();

[aeubanks]

llvm/include/llvm/IR/PassInstrumentation.h

150 ↗

(On Diff #276860)

Each pass instrumentation should decide whether or not to run the pass based on whether or not the pass is required or optional. An optional pass may still be run, (which should be the case for the vast majority of instances).

For example, the optnone would only care if a pass is required or not if it sees that a function is marked optnone.
Similarly, opt-bisect would only care if a pass is required if it's hit the bisect limit.

[aeubanks]

llvm/include/llvm/IR/PassInstrumentation.h

150 ↗

(On Diff #276860)

Sorry, now I understand what you mean, the ands and ors confused me.

I don't want to rule out the possibility of some future pass instrumentation wanting to skip even a required pass. But I am open to discussion on this point.

[ychen]

High-level request: how about split this patch into two, the first for the require pass part; the second for the PassInstrument callback. Then we could discuss the choices of first patch and D82344.

llvm/include/llvm/IR/PassInstrumentation.h

150 ↗

(On Diff #276860)

I don't want to rule out the possibility of some future pass instrumentation wanting to skip even a required pass. But I am open to discussion on this point.

That makes sense. However, since this requires changing the callback API(return value or parameter), and there is no real use of it for the moment. IMHO we should defer it to the use case comes up. If there was no change to the callback API, I wouldn't mind doing this for now.

The immediate motivation for the require is the same as D82344 (the approach is a little bit different). That's we don't want to consider infrastructure passes (pass managers, adaptor passes that have a nested pass manager)

[aeubanks]

In D83519#2144403, @ychen wrote:

High-level request: how about split this patch into two, the first for the require pass part; the second for the PassInstrument callback. Then we could discuss the choices of first patch and D82344.

Good idea, will split the patch and take a closer look at your patch.

llvm/include/llvm/IR/PassInstrumentation.h

150 ↗

(On Diff #276860)

Sounds good, I'll go with your approach for keeping the current API.

[aeubanks]

In D83519#2144463, @aeubanks wrote:

In D83519#2144403, @ychen wrote:

High-level request: how about split this patch into two, the first for the require pass part; the second for the PassInstrument callback. Then we could discuss the choices of first patch and D82344.

Good idea, will split the patch and take a closer look at your patch.

I split the required passes part into https://reviews.llvm.org/D83575.

[ychen]

In D83519#2144616, @aeubanks wrote:

In D83519#2144463, @aeubanks wrote:

In D83519#2144403, @ychen wrote:

High-level request: how about split this patch into two, the first for the require pass part; the second for the PassInstrument callback. Then we could discuss the choices of first patch and D82344.

Good idea, will split the patch and take a closer look at your patch.

I split the required passes part into https://reviews.llvm.org/D83575.

Could you remove the Required part of this patch?

[aeubanks]

Remove required passes part
Rebase

[aeubanks]

In D83519#2158938, @ychen wrote:

In D83519#2144616, @aeubanks wrote:

In D83519#2144463, @aeubanks wrote:

In D83519#2144403, @ychen wrote:

High-level request: how about split this patch into two, the first for the require pass part; the second for the PassInstrument callback. Then we could discuss the choices of first patch and D82344.

Good idea, will split the patch and take a closer look at your patch.

I split the required passes part into https://reviews.llvm.org/D83575.

Could you remove the Required part of this patch?

Done. The test now fails because your patch for required passes hasn't been submitted yet, after that's submitted this one should be good to go.

[ychen]

llvm/include/llvm/Passes/StandardInstrumentations.h

57

I may probably name this SkipPassInstrumentation. But it is up to you.

65

How about calling this DebugLogging?

llvm/lib/Passes/StandardInstrumentations.cpp

27

Why need this?

270

if (F && F->hasOptNone()) {
   ...
}
return true;

[aeubanks]

Address comments

[ychen]

LGTM with one nit.

llvm/include/llvm/Passes/StandardInstrumentations.h

57

Errr, let's go with OptNoneInstrumentation if it's your preference. This is a bit too verbose IMHO.

[aeubanks]

Rename OptNoneInstrumentation

[MaskRay]

llvm/include/llvm/Passes/StandardInstrumentations.h

77

(No need to change) PrintIR & TimePasses don't need to appear in the initializer list. They would be default initialized.

clang/lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   PTO.LoopVectorization = CodeGenOpts.VectorizeLoop;
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
   // Only enable CGProfilePass when using integrated assembler, since
   // non-integrated assemblers don't recognize .cgprofile section.
   PTO.CallGraphProfile = !CodeGenOpts.DisableIntegratedAS;
   PTO.Coroutines = LangOpts.Coroutines;
 
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI;
+  StandardInstrumentations SI(CodeGenOpts.DebugPassManager);
   SI.registerCallbacks(PIC);
   PassBuilder PB(TM.get(), PTO, PGOOpt, &PIC);
 
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto &PluginFN : CodeGenOpts.PassPlugins) {
     auto PassPlugin = PassPlugin::Load(PluginFN);
     if (PassPlugin) {
       PassPlugin->registerPassBuilderCallbacks(PB);

llvm/include/llvm/Passes/StandardInstrumentations.h

 /// (i.e in-tree) pass instrumentations.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_PASSES_STANDARDINSTRUMENTATIONS_H
 #define LLVM_PASSES_STANDARDINSTRUMENTATIONS_H
 
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PassTimingInfo.h"
 
 #include <string>
 #include <utility>
 
 namespace llvm {
 
private:
   PrintModuleDesc popModuleDesc(StringRef PassID);
 
   /// Stack of Module description, enough to print the module after a given
   /// pass.
   SmallVector<PrintModuleDesc, 2> ModuleDescStack;
   bool StoreModuleDesc = false;
 };
 
+class OptNoneInstrumentation {
+public:
+  OptNoneInstrumentation(bool DebugPM) : DebugPM(DebugPM) {}
+  void registerCallbacks(PassInstrumentationCallbacks &PIC);
+
+private:
+  bool skip(StringRef PassID, Any IR);
+
+  bool DebugPM;
+};
+
 /// This class provides an interface to register all the standard pass
 /// instrumentations and manages their state (if any).
 class StandardInstrumentations {
   PrintIRInstrumentation PrintIR;
   TimePassesHandler TimePasses;
+  OptNoneInstrumentation OptNone;
 
 public:
-  StandardInstrumentations() = default;
+  StandardInstrumentations(bool DebugPM)
+      : PrintIR(), TimePasses(), OptNone(DebugPM) {}
 
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
   TimePassesHandler &getTimePasses() { return TimePasses; }
 };
 } // namespace llvm
 
 #endif

llvm/lib/LTO/LTOBackend.cpp

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;
+  StandardInstrumentations SI(Conf.DebugPassManager);
   SI.registerCallbacks(PIC);
   PassBuilder PB(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");
 

llvm/lib/Passes/StandardInstrumentations.cpp

 //===- Standard pass instrumentations handling ----------------*- C++ -*--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 /// \file
 ///
 /// This file defines IR-printing pass instrumentation callbacks as well as
 /// StandardInstrumentations class that manages standard pass instrumentations.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Passes/StandardInstrumentations.h"
+#include "llvm/ADT/Any.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Analysis/CallGraphSCCPass.h"
 #include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassInstrumentation.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
+// TODO: remove once all required passes are marked as such.
+static cl::opt<bool>
+    EnableOptnone("enable-npm-optnone", cl::init(false),
+                  cl::desc("Enable skipping optional passes optnone functions "
+                           "under new pass manager"));
+
 namespace {
 
 /// Extracting Module out of \p IR unit. Also fills a textual description
 /// of \p IR for use in header when printing.
 Optional<std::pair<const Module *, std::string>> unwrapModule(Any IR) {
   if (any_isa<const Module *>(IR))
     return std::make_pair(any_cast<const Module *>(IR), std::string());
 
void PrintIRInstrumentation::registerCallbacks(
   if (llvm::shouldPrintAfterPass()) {
     PIC.registerAfterPassCallback(
         [this](StringRef P, Any IR) { this->printAfterPass(P, IR); });
     PIC.registerAfterPassInvalidatedCallback(
         [this](StringRef P) { this->printAfterPassInvalidated(P); });
   }
 }
 
+void OptNoneInstrumentation::registerCallbacks(
+    PassInstrumentationCallbacks &PIC) {
+  PIC.registerBeforePassCallback(
+      [this](StringRef P, Any IR) { return this->skip(P, IR); });
+}
+
+bool OptNoneInstrumentation::skip(StringRef PassID, Any IR) {
+  if (!EnableOptnone)
+    return true;
+  const Function *F = nullptr;
+  if (any_isa<const Function *>(IR)) {
+    F = any_cast<const Function *>(IR);
+  } else if (any_isa<const Loop *>(IR)) {
+    F = any_cast<const Loop *>(IR)->getHeader()->getParent();
+  }
+  if (F) {
+    bool HasOptNone = F->hasOptNone();
+    if (HasOptNone) {
+      if (DebugPM)
+        dbgs() << "Skipping pass: " << PassID << " (optnone)\n";
+      return false;
+    }
+  }
+  return true;
+}
+
 void StandardInstrumentations::registerCallbacks(
     PassInstrumentationCallbacks &PIC) {
   PrintIR.registerCallbacks(PIC);
   TimePasses.registerCallbacks(PIC);
+  OptNone.registerCallbacks(PIC);
 }

llvm/test/Feature/optnone-opt.ll

-; RUN: opt     -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O0
-; RUN: opt -O1 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1
-; RUN: opt -O2 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1 --check-prefix=OPT-O2O3
-; RUN: opt -O3 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1 --check-prefix=OPT-O2O3
-; RUN: opt -dce -die -gvn-hoist -loweratomic -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-MORE
-; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-LOOP
+; RUN: opt     -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O0
+; RUN: opt -O1 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1
+; RUN: opt -O2 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
+; RUN: opt -O3 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
+; RUN: opt -dce -die -gvn-hoist -loweratomic -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=MORE
+; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -enable-new-pm=0 -S -debug %s 2>&1 | FileCheck %s --check-prefix=LOOP
+; RUN: opt -enable-npm-optnone     -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O0
+; RUN: opt -enable-npm-optnone -O1 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1
+; RUN: opt -enable-npm-optnone -O2 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -enable-npm-optnone -O3 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -enable-npm-optnone -dce -gvn-hoist -loweratomic -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-MORE
+; RUN: opt -enable-npm-optnone -indvars -licm -loop-deletion -loop-idiom -loop-instsimplify -loop-reduce -loop-unroll -simple-loop-unswitch -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
 
 ; REQUIRES: asserts
 
 ; This test verifies that we don't run target independent IR-level
 ; optimizations on optnone functions.
 
 ; Function Attrs: noinline optnone
 define i32 @_Z3fooi(i32 %x) #0 {

 
 while.end:                                        ; preds = %while.cond
   ret i32 0
 }
 
 attributes #0 = { optnone noinline }
 
 ; Nothing that runs at -O0 gets skipped.
-; OPT-O0-NOT: Skipping pass
+; O0-NOT: Skipping pass
+; NPM-O0-NOT: Skipping pass
 
 ; IR passes run at -O1 and higher.
-; OPT-O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
-; OPT-O1-DAG: Skipping pass 'Combine redundant instructions'
-; OPT-O1-DAG: Skipping pass 'Early CSE'
-; OPT-O1-DAG: Skipping pass 'Reassociate expressions'
-; OPT-O1-DAG: Skipping pass 'Simplify the CFG'
-; OPT-O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'
+; O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
+; O1-DAG: Skipping pass 'Combine redundant instructions'
+; O1-DAG: Skipping pass 'Early CSE'
+; O1-DAG: Skipping pass 'Reassociate expressions'
+; O1-DAG: Skipping pass 'Simplify the CFG'
+; O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'
+; NPM-O1-DAG: Skipping pass: SimplifyCFGPass
+; NPM-O1-DAG: Skipping pass: SROA
+; NPM-O1-DAG: Skipping pass: EarlyCSEPass
+; NPM-O1-DAG: Skipping pass: LowerExpectIntrinsicPass
+; NPM-O1-DAG: Skipping pass: PromotePass
+; NPM-O1-DAG: Skipping pass: InstCombinePass
 
 ; Additional IR passes run at -O2 and higher.
-; OPT-O2O3-DAG: Skipping pass 'Global Value Numbering'
-; OPT-O2O3-DAG: Skipping pass 'SLP Vectorizer'
+; O2O3-DAG: Skipping pass 'Global Value Numbering'
+; O2O3-DAG: Skipping pass 'SLP Vectorizer'
+; NPM-O2O3-DAG: Skipping pass: GVN
+; NPM-O2O3-DAG: Skipping pass: SLPVectorizerPass
 
 ; Additional IR passes that opt doesn't turn on by default.
-; OPT-MORE-DAG: Skipping pass 'Dead Code Elimination'
-; OPT-MORE-DAG: Skipping pass 'Dead Instruction Elimination'
+; MORE-DAG: Skipping pass 'Dead Code Elimination'
+; MORE-DAG: Skipping pass 'Dead Instruction Elimination'
+; NPM-MORE-DAG: Skipping pass: GVNHoistPass
+; NPM-MORE-DAG: Skipping pass: LowerAtomicPass
 
 ; Loop IR passes that opt doesn't turn on by default.
-; OPT-LOOP-DAG: Skipping pass 'Delete dead loops'
-; OPT-LOOP-DAG: Skipping pass 'Induction Variable Simplification'
-; OPT-LOOP-DAG: Skipping pass 'Loop Invariant Code Motion'
-; OPT-LOOP-DAG: Skipping pass 'Loop Strength Reduction'
-; OPT-LOOP-DAG: Skipping pass 'Recognize loop idioms'
-; OPT-LOOP-DAG: Skipping pass 'Reroll loops'
-; OPT-LOOP-DAG: Skipping pass 'Rotate Loops'
-; OPT-LOOP-DAG: Skipping pass 'Simplify instructions in loops'
-; OPT-LOOP-DAG: Skipping pass 'Unroll loops'
-; OPT-LOOP-DAG: Skipping pass 'Unswitch loops'
+; LOOP-DAG: Skipping pass 'Delete dead loops'
+; LOOP-DAG: Skipping pass 'Induction Variable Simplification'
+; LOOP-DAG: Skipping pass 'Loop Invariant Code Motion'
+; LOOP-DAG: Skipping pass 'Loop Strength Reduction'
+; LOOP-DAG: Skipping pass 'Recognize loop idioms'
+; LOOP-DAG: Skipping pass 'Reroll loops'
+; LOOP-DAG: Skipping pass 'Rotate Loops'
+; LOOP-DAG: Skipping pass 'Simplify instructions in loops'
+; LOOP-DAG: Skipping pass 'Unroll loops'
+; LOOP-DAG: Skipping pass 'Unswitch loops'
+; NPM-LOOP-DAG: Skipping pass: LoopSimplifyPass
+; NPM-LOOP-DAG: Skipping pass: LCSSAPass
+; NPM-LOOP-DAG: Skipping pass: IndVarSimplifyPass
+; NPM-LOOP-DAG: Skipping pass: SimpleLoopUnswitchPass
+; NPM-LOOP-DAG: Skipping pass: LoopUnrollPass
+; NPM-LOOP-DAG: Skipping pass: LoopStrengthReducePass
+; NPM-LOOP-DAG: Skipping pass: LoopDeletionPass
+; NPM-LOOP-DAG: Skipping pass: LICMPass
+; NPM-LOOP-DAG: Skipping pass: LoopIdiomRecognizePass
+; NPM-LOOP-DAG: Skipping pass: LoopInstSimplifyPass

llvm/tools/opt/NewPMDriver.cpp

if (CSPGOKindFlag == CSInstrGen) {
                        PGOOptions::NoAction, PGOOptions::CSIRInstr);
     } else /* CSPGOKindFlag == CSInstrUse */ {
       if (!P)
         errs() << "CSInstrUse needs to be together with InstrUse";
       P->CSAction = PGOOptions::CSIRUse;
     }
   }
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI;
+  StandardInstrumentations SI(DebugPM);
   SI.registerCallbacks(PIC);
 
   PipelineTuningOptions PTO;
   // LoopUnrolling defaults on to true and DisableLoopUnrolling is initialized
   // to false above so we shouldn't necessarily need to check whether or not the
   // option has been enabled.
   PTO.LoopUnrolling = !DisableLoopUnrolling;
   PTO.Coroutines = Coroutines;