Introduce -print-before-changed, making -print-changed also print before passes that modify IR

[jamieschmeiser]

Summary

Add an option -print-before-changed that modifies the print-changed
behaviour so that it prints the IR before a pass that changed it in
addition to printing the IR after the pass. Note that the option
does nothing in isolation. The filtering options work as expected.
Lit tests are included.

[aeubanks]

Could you capitalize and shorten the title a bit? Something like "Introduce -print-before-changed, making -print-changed also print before passes that modify IR"

[jamieschmeiser]

Changed name of option to -also-print-before to avoid common option prefix problem.

[jamieschmeiser]

@aeubanks, just want to point out that I have changed the option name to avoid the problem of -print-before and -print-before-changed having a common prefix. The option name is now -also-print-before. Please verify that this is acceptable and then I will deliver this.

[aeubanks]

I don't like -also-print-before, it's too generic. I'd prefer something like -print-before-changed.

[MaskRay]

In D88757#2347711, @jamieschmeiser wrote:

Changed name of option to -also-print-before to avoid common option prefix problem.

Please fix the revision subject as well.

[jamieschmeiser]

@aeubanks, I agree but when I first put up this PR, there was a lit test failure and looking at the run command for the test, it had "/fuchsia.cpp -### -no-canonical-prefixes" in it, so I think the problem is that the existing -print-before is a prefix of -print-before-changed. How about -also-print-before-changed?

[aeubanks]

In D88757#2347769, @jamieschmeiser wrote:

@aeubanks, I agree but when I first put up this PR, there was a lit test failure and looking at the run command for the test, it had "/fuchsia.cpp -### -no-canonical-prefixes" in it, so I think the problem is that the existing -print-before is a prefix of -print-before-changed. How about -also-print-before-changed?

I thought -### added quotes to not have this sort of issue.
What exactly is the test failure?

[jamieschmeiser]

@aeubanks, you can see the failure report by looking at Diff 1 in the history tab of the revision contents in this PR (https://reviews.llvm.org/D88757?id=295916):

linux > Clang.Driver::fuchsia.cpp

Script:

: 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang --driver-mode=g++ /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/fuchsia.cpp -### -no-canonical-prefixes --target=x86_64-fuchsia -ccc-install-dir /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/Inputs/basic_fuchsia_tree/bin -resource-dir=/mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/Inputs/resource_dir_with_per_target_subdir --sysroot=/mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/platform -fuse-ld=lld 2>&1 | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck -check-prefixes=CHECK,CHECK-X86_64 /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/fuchsia.cpp

[aeubanks]

That failure doesn't look related to this change, looks like something else broke it. Can you repro locally?

[jamieschmeiser]

@aeubanks, it appears that you are correct that the previous error was not caused by my changes as I cannot reproduce it locally. I have changed the option back to -print-before-changed.

[aeubanks]

lgtm

llvm/lib/Passes/StandardInstrumentations.cpp

 // The specified passes will either be reported as making no changes (with
 // no IR reported) or the changed IR will be reported.  Also, the
 // -filter-print-funcs and -print-module-scope options will do similar
 // filtering based on function name, reporting changed IRs as functions(or
 // modules if -print-module-scope is specified) for a particular function
 // or indicating that the IR has been filtered out.  The extra options
 // can be combined, allowing only changed IRs for certain passes on certain
 // functions to be reported in different formats, with the rest being
-// reported as filtered out.
+// reported as filtered out.  The -print-before-changed option will print
+// the IR as it was before each pass that changed it.
 static cl::opt<bool> PrintChanged("print-changed",
                                   cl::desc("Print changed IRs"),
                                   cl::init(false), 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::list<std::string>
     PrintPassesList("filter-passes", cl::value_desc("pass names"),
                     cl::desc("Only consider IR changes for passes whose names "
                              "match for the print-changed option"),
                     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);
 
 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 IRChangePrinter::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) {
+    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());

llvm/test/Other/change-printer.ll

 ;
 ; Check that the reporting of IRs respects -filter-passes, -filter-print-funcs and -print-module-scope
 ; RUN: opt -S -print-changed -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-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 -passes="instsimplify,instsimplify" -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-MULT-PASSES-FILTER-FUNC
+;
+; Simple print-before-changed functionality check.
+; 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
+;
 
 define i32 @g() {
 entry:
   %a = add i32 2, 3
   ret i32 %a
 }
 
 define i32 @f() {

 
 ; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump At Start: ***
 ; CHECK-MULT-PASSES-FILTER-FUNC-NEXT: ; ModuleID = {{.+}}
 ; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump After InstSimplifyPass (function: g) filtered out ***
 ; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump After InstSimplifyPass (function: g) filtered out ***
 ; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump After InstSimplifyPass *** (function: f)
 ; CHECK-MULT-PASSES-FILTER-FUNC-NEXT: define i32 @f()
 ; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump After InstSimplifyPass (function: f) omitted because no change ***
+
+; CHECK-SIMPLE-BEFORE: *** IR Dump At Start: ***
+; CHECK-SIMPLE-BEFORE-NEXT: ; ModuleID = {{.+}}
+; CHECK-SIMPLE-BEFORE: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-SIMPLE-BEFORE: *** IR Dump Before InstSimplifyPass *** (function: g)
+; CHECK-SIMPLE-BEFORE-NEXT: define i32 @g()
+; CHECK-SIMPLE-BEFORE: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-SIMPLE-BEFORE-NEXT: define i32 @g()
+; CHECK-SIMPLE-BEFORE: *** IR Pass PassManager{{.*}} (function: g) ignored ***
+; CHECK-SIMPLE-BEFORE: *** IR Dump Before InstSimplifyPass *** (function: f)
+; CHECK-SIMPLE-BEFORE-NEXT: define i32 @f()
+; CHECK-SIMPLE-BEFORE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-SIMPLE-BEFORE-NEXT: define i32 @f()
+; CHECK-SIMPLE-BEFORE: *** IR Pass PassManager{{.*}} (function: f) ignored ***
+; CHECK-SIMPLE-BEFORE: *** IR Pass ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> (module) ignored ***
+; CHECK-SIMPLE-BEFORE: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-SIMPLE-BEFORE: *** IR Dump After PrintModulePass (module) omitted because no change ***
+; CHECK-SIMPLE-BEFORE-NOT: *** IR
+
+; CHECK-FUNC-FILTER-BEFORE: *** IR Dump At Start: ***
+; CHECK-FUNC-FILTER-BEFORE-NEXT: ; ModuleID = {{.+}}
+; CHECK-FUNC-FILTER-BEFORE: *** IR Dump After InstSimplifyPass (function: g) filtered out ***
+; CHECK-FUNC-FILTER-BEFORE: *** IR Dump Before InstSimplifyPass *** (function: f)
+; CHECK-FUNC-FILTER-BEFORE-NEXT: define i32 @f()
+; CHECK-FUNC-FILTER-BEFORE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-FUNC-FILTER-BEFORE-NEXT: define i32 @f()
+
+; CHECK-PRINT-MOD-SCOPE-BEFORE: *** IR Dump At Start: ***
+; CHECK-PRINT-MOD-SCOPE-BEFORE-NEXT: ModuleID = {{.+}}
+; 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 = {{.+}}