Introduce new quiet mode and new option handling for -print-changed.

[jamieschmeiser]

Summary

Introduce new quiet mode and new option handling for -print-changed.

Introduce a new mode of operation for -print-changed that only reports
after a pass changes the IR with all of the other messages suppressed (ie,
no initial IR and no messages about ignored, filtered or non-modifying
passes).

The option processing for -print-changed is changed to take an optional
string indicating options for print-changed. Initially, the only option
supported is quiet (as described above). This new quiet mode is specified
with -print-changed=quiet while -print-changed will continue to function
in the same way. It is intended that there will be more options in the
future.

[aeubanks]

Thanks, this is much more reviewable.

Should this be a flag toggle for print-changed? You'd never run -print-changed and -print-changed-only at the same time right? Perhaps defaulting to this for -print-changed, and adding a "verbose" option which would preserve the existing behavior would be nice. So it'd be something like the existing -debug-pass-manager-verbose flag. I vaguely remember using the -print-changed flag (I do like it a lot :) ) and being slightly annoyed with the verbosity of the ignored/special passes. WDYT?

[jamieschmeiser]

I was thinking about a slightly different approach for these options. There are several different options (-print-changed, print-changed-diff, -dot-cfg-changed ) with different variations. As you say, it doesn't make sense to use more than one at a time so I was thinking that perhaps there could be -print-changed, and -print-changed-options=<options>. Without -print-changed-options, you would get this one (ie, before and after without banners for no changed, ignored, etc). The options would be a string of characters v being verbose (all banners), b is -print-before-changed, p is patch format (-print-changed-diff which could also have a verbose mode as currently up for review and default to skipping other banners). -dot-cfg-changed (also with/without verbose mode) would be d=string, b=string, a=string, c=string with strings going to the end of input or comma where d, b, a and c being the directory, before colour, after colour and common colour. The option letters can change, the important thing is the idea of the first option saying you want some form of change reporter (defaulting to this nonverbose print-changed version) and a modifier option that picks and controls the various types of reporter. WDYT?

[aeubanks]

I like that idea. Actually I think you might even be able to fit it all in one flag. Just passing -print-changed gives you the default, and you can say something like -print-changed=quiet to not print on skipped passes (as an example). And apparently this is possible with cl::opt, e.g. I found

static cl::opt<RunOutliner> EnableMachineOutliner(
    "enable-machine-outliner", cl::desc("Enable the machine outliner"),
    cl::Hidden, cl::ValueOptional, cl::init(TargetDefault),
    cl::values(clEnumValN(AlwaysOutline, "always",
                          "Run on all functions guaranteed to be beneficial"),
               clEnumValN(NeverOutline, "never", "Disable all outlining"),
               // Sentinel value for unspecified option.
               clEnumValN(AlwaysOutline, "", "")));

[jamieschmeiser]

I have changed the option handling as discussed and used the optional
string on the option as you suggested. I have also changed this from
being a separate change printer to being an option on the base class.
This way, they other proposed change printers can also benefit by getting
the verbose/quiet modes using the same code.

[aeubanks]

LGTM!

llvm/include/llvm/Passes/StandardInstrumentations.h

 };
 
 // A change printer based on the string representation of the IR as created
 // by unwrapAndPrint.  The string representation is stored in a std::string
 // to preserve it as the IR changes in each pass.  Note that the banner is
 // included in this representation but it is massaged before reporting.
 class IRChangedPrinter : public TextChangeReporter<std::string> {
 public:
-  IRChangedPrinter() {}
+  IRChangedPrinter(bool PrintBefore) : PrintBefore(PrintBefore) {}
   ~IRChangedPrinter() override;
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
 protected:
   // Called before and after a pass to get the representation of the IR.
   void generateIRRepresentation(Any IR, StringRef PassID,
                                 std::string &Output) override;
   // Called when an interesting IR has changed.
   void handleAfter(StringRef PassID, std::string &Name,
                    const std::string &Before, const std::string &After,
                    Any) override;
   // Called to compare the before and after representations of the IR.
   bool same(const std::string &Before, const std::string &After) override;
+
+  bool PrintBefore;
+};
+
+// A change printer that only reports the before and after IR representations
+// when the IR changes, suppressing all other messages, including the
+// initial IR.  Normal filtering options for change reporters apply.
+class IRChangedOnlyPrinter : public IRChangedPrinter {
+public:
+  IRChangedOnlyPrinter() : IRChangedPrinter(true) {}
+  ~IRChangedOnlyPrinter() override;
+  void registerCallbacks(PassInstrumentationCallbacks &PIC);
+
+protected:
+  // Suppress these messages
+  void handleInitialIR(Any IR) override;
+  void omitAfter(StringRef PassID, std::string &Name) override;
+  void handleInvalidated(StringRef PassID) override;
+  void handleFiltered(StringRef PassID, std::string &Name) override;
+  void handleIgnored(StringRef PassID, std::string &Name) override;
 };
 
 class VerifyInstrumentation {
   bool DebugLogging;
 
 public:
   VerifyInstrumentation(bool DebugLogging) : DebugLogging(DebugLogging) {}
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 };
 
 /// This class provides an interface to register all the standard pass
 /// instrumentations and manages their state (if any).
 class StandardInstrumentations {
   PrintIRInstrumentation PrintIR;
   PrintPassInstrumentation PrintPass;
   TimePassesHandler TimePasses;
   OptNoneInstrumentation OptNone;
   OptBisectInstrumentation OptBisect;
   PreservedCFGCheckerInstrumentation PreservedCFGChecker;
   IRChangedPrinter PrintChangedIR;
+  IRChangedOnlyPrinter PrintChangedIROnly;
   VerifyInstrumentation Verify;
 
   bool VerifyEach;
 
 public:
-  StandardInstrumentations(bool DebugLogging, bool VerifyEach = false)
-      : PrintPass(DebugLogging), OptNone(DebugLogging), Verify(DebugLogging),
-        VerifyEach(VerifyEach) {}
+  StandardInstrumentations(bool DebugLogging, bool VerifyEach = false);
 
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
   TimePassesHandler &getTimePasses() { return TimePasses; }
 };
 
 extern template class ChangeReporter<std::string>;
 extern template class TextChangeReporter<std::string>;
 
 } // namespace llvm
 
 #endif

llvm/lib/Passes/StandardInstrumentations.cpp

PrintPassesList("filter-passes", cl::value_desc("pass names"),
                     cl::CommaSeparated, cl::Hidden);
 // An option that supports the -print-changed option.  See
 // the description for -print-changed for an explanation of the use
 // of this option.  Note that this option has no effect without -print-changed.
 static cl::opt<bool>
     PrintChangedBefore("print-before-changed",
                        cl::desc("Print before passes that change them"),
                        cl::init(false), cl::Hidden);
+// A change printer that only prints the before and after version of the IR
+// when the IR changes.  All messages about non-reporting passes and the
+// initial IR are suppressed.  Output is modified by the usual modifying
+// options (filter-print-funcs, filter-passes, print-module-scope).
+static cl::opt<bool>
+    PrintChangedOnly("print-changed-only",
+                     cl::desc("Only print before and after passes that"
+                              "change them"),
+                     cl::init(false), cl::Hidden);
 
 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, bool Force = false) {
   if (any_isa<const Module *>(IR))
void IRChangedPrinter::handleAfter(StringRef PassID, std::string &Name,
                                    const std::string &Before,
                                    const std::string &After, Any) {
   assert(After.find("*** IR Dump") == 0 && "Unexpected banner format.");
   StringRef AfterRef = After;
   StringRef Banner =
       AfterRef.take_until([](char C) -> bool { return C == '\n'; });
 
   // Report the IR before the changes when requested.
-  if (PrintChangedBefore) {
+  if (PrintBefore) {
     Out << "*** IR Dump Before" << Banner.substr(17);
     // LazyCallGraph::SCC already has "(scc:..." in banner so only add
     // in the name if it isn't already there.
     if (Name.substr(0, 6) != " (scc:" && !llvm::forcePrintModuleIR())
       Out << Name;
 
     StringRef BeforeRef = Before;
     Out << BeforeRef.substr(Banner.size());
   }
 
   Out << Banner;
 
   // LazyCallGraph::SCC already has "(scc:..." in banner so only add
   // in the name if it isn't already there.
   if (Name.substr(0, 6) != " (scc:" && !llvm::forcePrintModuleIR())
     Out << Name;
 
   Out << After.substr(Banner.size());
 }
 
 bool IRChangedPrinter::same(const std::string &S1, const std::string &S2) {
   return S1 == S2;
 }
 
+IRChangedOnlyPrinter::~IRChangedOnlyPrinter() {}
+void IRChangedOnlyPrinter::registerCallbacks(
+    PassInstrumentationCallbacks &PIC) {
+  if (PrintChangedOnly)
+    TextChangeReporter<std::string>::registerRequiredCallbacks(PIC);
+}
+// Suppress these messages for this change reporter.
+void IRChangedOnlyPrinter::handleInitialIR(Any) {}
+void IRChangedOnlyPrinter::omitAfter(StringRef, std::string &) {}
+void IRChangedOnlyPrinter::handleInvalidated(StringRef) {}
+void IRChangedOnlyPrinter::handleFiltered(StringRef, std::string &) {}
+void IRChangedOnlyPrinter::handleIgnored(StringRef, std::string &) {}
+
 PrintIRInstrumentation::~PrintIRInstrumentation() {
   assert(ModuleDescStack.empty() && "ModuleDescStack is not empty at exit");
 }
 
 void PrintIRInstrumentation::pushModuleDesc(StringRef PassID, Any IR) {
   assert(StoreModuleDesc);
   const Module *M = nullptr;
   std::string Extra;
PIC.registerAfterPassCallback(
             dbgs() << "Verifying module " << M->getName() << "\n";
 
           if (verifyModule(*M))
             report_fatal_error("Broken module found, compilation aborted!");
         }
       });
 }
 
+StandardInstrumentations::StandardInstrumentations(bool DebugLogging,
+                                                   bool VerifyEach)
+    : PrintPass(DebugLogging), OptNone(DebugLogging),
+      PrintChangedIR(PrintChangedBefore), Verify(DebugLogging),
+      VerifyEach(VerifyEach) {}
+
 void StandardInstrumentations::registerCallbacks(
     PassInstrumentationCallbacks &PIC) {
   PrintIR.registerCallbacks(PIC);
   PrintPass.registerCallbacks(PIC);
   TimePasses.registerCallbacks(PIC);
   OptNone.registerCallbacks(PIC);
   OptBisect.registerCallbacks(PIC);
   PreservedCFGChecker.registerCallbacks(PIC);
   PrintChangedIR.registerCallbacks(PIC);
+  PrintChangedIROnly.registerCallbacks(PIC);
   if (VerifyEach)
     Verify.registerCallbacks(PIC);
 }
 
 namespace llvm {
 
 template class ChangeReporter<std::string>;
 template class TextChangeReporter<std::string>;
 
 } // namespace llvm

llvm/test/Other/change-printer.ll

 ; RUN: opt -S -print-changed -print-before-changed -passes=instsimplify 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-SIMPLE-BEFORE
 ;
 ; Check print-before-changed obeys the function filtering
 ; RUN: opt -S -print-changed -print-before-changed -passes=instsimplify -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FUNC-FILTER-BEFORE
 ;
 ; Check that the reporting of IRs with -print-before-changed respects -print-module-scope
 ; RUN: opt -S -print-changed -print-before-changed -passes=instsimplify -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-PRINT-MOD-SCOPE-BEFORE
 ;
+; Simple checks of -print-changed-only functionality
+;
+; Simple functionality check.
+; RUN: opt -S -print-changed-only -passes=instsimplify 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-SIMPLE
+;
+; Check that only the passes that change the IR are printed and that the
+; others (including g) are filtered out.
+; RUN: opt -S -print-changed-only -passes=instsimplify -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FUNC-FILTER
+;
+; Check that the reporting of IRs respects -print-module-scope
+; RUN: opt -S -print-changed-only -passes=instsimplify -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE
+;
+; Check that the reporting of IRs respects -print-module-scope
+; RUN: opt -S -print-changed-only -passes=instsimplify -filter-print-funcs=f -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FUNC-FILTER-MOD-SCOPE
+;
+; Check that reporting of multiple functions happens
+; RUN: opt -S -print-changed-only -passes=instsimplify -filter-print-funcs="f,g" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FILTER-MULT-FUNC
+;
+; Check that the reporting of IRs respects -filter-passes
+; RUN: opt -S -print-changed-only -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FILTER-PASSES-NONE --allow-empty
+;
+; Check that the reporting of IRs respects -filter-passes with multiple passes
+; RUN: opt -S -print-changed-only -passes="instsimplify" -filter-passes="NoOpFunctionPass,InstSimplifyPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FILTER-PASSES
+;
+; Check that the reporting of IRs respects -filter-passes with multiple passes
+; RUN: opt -S -print-changed-only -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FILTER-MULT-PASSES
+;
+; Check that the reporting of IRs respects both -filter-passes and -filter-print-funcs
+; RUN: opt -S -print-changed-only -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" -filter-print-funcs=f 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES
+;
+; Check that the reporting of IRs respects -filter-passes, -filter-print-funcs and -print-module-scope
+; RUN: opt -S -print-changed-only -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" -filter-print-funcs=f -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-MOD-SCOPE
+;
+; Check that repeated passes that change the IR are printed and that the
+; others (including g) are filtered out.  Note that the second time
+; instsimplify is run on f, it does not change the IR
+; RUN: opt -S -print-changed-only -passes="instsimplify,instsimplify" -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-CHANGED-ONLY-MULT-PASSES-FILTER-FUNC
+;
 
 define i32 @g() {
 entry:
   %a = add i32 2, 3
   ret i32 %a
 }
 
 define i32 @f() {

 ; CHECK-PRINT-MOD-SCOPE-BEFORE: *** IR Dump Before InstSimplifyPass *** (function: g)
 ; CHECK-PRINT-MOD-SCOPE-BEFORE-NEXT: ModuleID = {{.+}}
 ; CHECK-PRINT-MOD-SCOPE-BEFORE: *** IR Dump After InstSimplifyPass *** (function: g)
 ; CHECK-PRINT-MOD-SCOPE-BEFORE-NEXT: ModuleID = {{.+}}
 ; CHECK-PRINT-MOD-SCOPE-BEFORE: *** IR Dump Before InstSimplifyPass *** (function: f)
 ; CHECK-PRINT-MOD-SCOPE-BEFORE-NEXT: ModuleID = {{.+}}
 ; CHECK-PRINT-MOD-SCOPE-BEFORE: *** IR Dump After InstSimplifyPass *** (function: f)
 ; CHECK-PRINT-MOD-SCOPE-BEFORE-NEXT: ModuleID = {{.+}}
+
+; CHECK-CHANGED-ONLY-SIMPLE-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-SIMPLE: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-CHANGED-ONLY-SIMPLE-NEXT: define i32 @g()
+; CHECK-CHANGED-ONLY-SIMPLE-NOT: *** IR Dump {{.*(no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-SIMPLE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-SIMPLE-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-SIMPLE-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FUNC-FILTER-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FUNC-FILTER: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FUNC-FILTER-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-FUNC-FILTER-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE-NEXT: ModuleID = {{.+}}
+; CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE-NEXT: ModuleID = {{.+}}
+; CHECK-CHANGED-ONLY-PRINT-MOD-SCOPE-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FUNC-FILTER-MOD-SCOPE-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FUNC-FILTER-MOD-SCOPE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FUNC-FILTER-MOD-SCOPE-NEXT: ModuleID = {{.+}}
+; CHECK-CHANGED-ONLY-FUNC-FILTER-MOD-SCOPE-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FILTER-MULT-FUNC-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-MULT-FUNC: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-CHANGED-ONLY-FILTER-MULT-FUNC-NEXT: define i32 @g()
+; CHECK-CHANGED-ONLY-FILTER-MULT-FUNC: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FILTER-MULT-FUNC-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-FILTER-MULT-FUNC-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FILTER-PASSES-NONE-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FILTER-PASSES-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-PASSES: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-CHANGED-ONLY-FILTER-PASSES-NEXT: define i32 @g()
+; CHECK-CHANGED-ONLY-FILTER-PASSES-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-PASSES: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FILTER-PASSES-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-FILTER-PASSES-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES-NEXT: define i32 @g()
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-FILTER-MULT-PASSES-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-MOD-SCOPE-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-MOD-SCOPE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-MOD-SCOPE-NEXT: ModuleID = {{.+}}
+; CHECK-CHANGED-ONLY-FILTER-FUNC-PASSES-MOD-SCOPE-NOT: *** IR
+
+; CHECK-CHANGED-ONLY-MULT-PASSES-FILTER-FUNC-NOT: *** IR Dump {{.*(At Start:|no change|ignored|filtered out)}} ***
+; CHECK-CHANGED-ONLY-MULT-PASSES-FILTER-FUNC: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-CHANGED-ONLY-MULT-PASSES-FILTER-FUNC-NEXT: define i32 @f()
+; CHECK-CHANGED-ONLY-MULT-PASSES-FILTER-FUNC-NOT: *** IR