[NFC intended] Refactor code for print-changed to facilitate reuse.

[jamieschmeiser]

Summary

[NFC intended] Refactor the code for printChanged for reuse and to facilitate
subsequent reporters of changes to the IR in the new pass manager.

Create abstract template base classes for common functionality and give
classes more appropriate names.  The base classes handle all of the
determination of when a function or pass is "interesting" and should be
reported or filtered out. They have pure virtual functions which are called
when a change by a pass has been recognized so the derived class need only
provide the overrides to present the information about the changing IR.
There are at least 2 more change reporters to come (which were presented
in my tutorial at the 2020 llvm developer's meeting) that derive from
these classes.

Respond to review comments: move function out of line, remove inline keyword,

remove unneeded qualifiers, simplify comparison.

[jamieschmeiser]

Note that this code makes use of the linux diff function to determine what has changed in the IR. This will likely require altering for other platforms, which is a future enhancement. The testing may require attention before this PR can land and any suggestions on how to handle this would be appreciated.

[jamieschmeiser]

Rebase to pick up review changes for https://reviews.llvm.org/D86360 and
alter this code to match those changes (reordered parameters, added const
to parameters)

[jamieschmeiser]

This is an update based on changes that have been made to the code in the review
for https://reviews.llvm.org/D86360. I have moved much of the code and templates into
a new header and body files as it is becoming more complex. I have also changed
some of the class names from being "change printer" based to "change reporter"
based because it doesn't imply how the changes are shown (ie, the changes do not
necessarily need to be printed).

[jamieschmeiser]

Move enum out of template, make space between end template delimiters
optional in new test.

[madhur13490]

llvm/include/llvm/IR/ChangeReporters.h

258 ↗

(On Diff #290040)

Typo: compare

llvm/lib/IR/ChangeReporters.cpp

150 ↗

(On Diff #290040)

You are not using this variable.

187 ↗

(On Diff #290040)

can combine above two "if"s with logical or and return false.

llvm/lib/Passes/StandardInstrumentations.cpp

201

Why not use standard C++ file handling? This is old C's way of manipulating files.

225

I can be unsigned.

[jamieschmeiser]

Update this PR to match changes made to https://reviews.llvm.org/D86360,
most notably replace the lambdas with virtual functions.
Adapt to other changes based on reviews of -print-changed.

Respond to review comments on this PR: various requested changes.
Remove change-printer.ll because tests are now in change-reporters.ll

[jamieschmeiser]

Update PR based on print-changed which has now landed.
Do not move the code into a different file as I previously did
as it made it difficult to see the changes.

[jamieschmeiser]

Improved comments, changed order of classes in header file,
changed error handling to produce error messages rather
than asserting when performing the linux diff operation.

[jamieschmeiser]

Removed unnecessary template parameter and improved testing.

[jamieschmeiser]

ping

[jamieschmeiser]

Moved the tests into a separate directory and added a lit.local.cfg that tests
whether the needed diff support is available. The tests are flagged as
unsupported when the needed support is unavailable. Also, the tests
were expanded to check more of the results.

[aeubanks]

This is a really big change and it looks like there are multiple things happening: some refactoring and the actual diff printer, could you split this up so it's more manageable to review?

[jamieschmeiser]

As requested, I have broken out the refactoring aspects of the original PR.
I have placed them in this PR and will make a subsequent PR with the new
functionality of print-changes once these refactoring changes have
stabilized/landed to avoid churn in a chain of PRs. There are no intended
changes to functionality in this PR.

[aeubanks]

lg aside from some nits

llvm/include/llvm/Passes/StandardInstrumentations.h

166

can you move the definition to the .cpp file? Also, the llvm:: namespace shouldn't be necessary (throughout this entire patch)

183

are the inlines necessary?

llvm/lib/Passes/StandardInstrumentations.cpp

484

Before == After seems simpler

[jamieschmeiser]

Respond to review comments: move function out of line, remove inline keyword,
remove unneeded qualifiers, simplify comparison.

[aeubanks]

llvm/lib/Passes/StandardInstrumentations.cpp

310

I think I prefer Name.empty(), but up to you

[jamieschmeiser]

Rebasing code on latest master before delivering

llvm/include/llvm/Passes/StandardInstrumentations.h

 // 2.  To get the representation of the IR (of type \p T).
 // 3.  When a pass does not change the IR.
 // 4.  When a pass changes the IR (given both before and after representations
 //         of type \p T).
 // 5.  When an IR is invalidated.
 // 6.  When a pass is run on an IR that is not interesting (based on options).
 // 7.  When a pass is ignored (pass manager or adapter pass).
 // 8.  To compare two IR representations (of type \p T).
-template <typename IRUnitT> class ChangePrinter {
+template <typename IRUnitT> class ChangeReporter {
 protected:
-  ChangePrinter() {}
+  ChangeReporter() {}
 
 public:
-  virtual ~ChangePrinter();
+  virtual ~ChangeReporter();
 
   // Determine if this pass/IR is interesting and if so, save the IR
   // otherwise it is left on the stack without data.
   void saveIRBeforePass(Any IR, StringRef PassID);
   // Compare the IR from before the pass after the pass.
   void handleIRAfterPass(Any IR, StringRef PassID);
   // Handle the situation where a pass is invalidated.
   void handleInvalidatedPass(StringRef PassID);
 
 protected:
+  // Register required callbacks.
+  void registerRequiredCallbacks(PassInstrumentationCallbacks &PIC);
+
+  // Return true when this is a defined function for which printing
+  // of changes is desired.
+  bool isInterestingFunction(const Function &F);
+
+  // Return true when this is a pass for which printing of changes is desired.
+  bool isInterestingPass(StringRef PassID);
+
+  // Return true when this is a pass on IR for which printing
+  // of changes is desired.
+  bool isInteresting(Any IR, StringRef PassID);
+
   // Called on the first IR processed.
   virtual void handleInitialIR(Any IR) = 0;
   // Called before and after a pass to get the representation of the IR.
   virtual void generateIRRepresentation(Any IR, StringRef PassID,
                                         IRUnitT &Output) = 0;
   // Called when the pass is not iteresting.
   virtual void omitAfter(StringRef PassID, std::string &Name) = 0;
   // Called when an interesting IR has changed.
   virtual void handleAfter(StringRef PassID, std::string &Name,
                            const IRUnitT &Before, const IRUnitT &After,
                            Any) = 0;
   // Called when an interesting pass is invalidated.
   virtual void handleInvalidated(StringRef PassID) = 0;
   // Called when the IR or pass is not interesting.
   virtual void handleFiltered(StringRef PassID, std::string &Name) = 0;
   // Called when an ignored pass is encountered.
   virtual void handleIgnored(StringRef PassID, std::string &Name) = 0;
   // Called to compare the before and after representations of the IR.
   virtual bool same(const IRUnitT &Before, const IRUnitT &After) = 0;
 
   // Stack of IRs before passes.
   std::vector<IRUnitT> BeforeStack;
   // Is this the first IR seen?
   bool InitialIR = true;
 };
 
+// An abstract template base class that handles printing banners and
+// reporting when things have not changed or are filtered out.
+template <typename IRUnitT>
+class TextChangeReporter : public ChangeReporter<IRUnitT> {
+protected:
+  TextChangeReporter();
+
+  // Print a module dump of the first IR that is changed.
+  void handleInitialIR(Any IR) override;
+  // Report that the IR was omitted because it did not change.
+  void omitAfter(StringRef PassID, std::string &Name) override;
+  // Report that the pass was invalidated.
+  void handleInvalidated(StringRef PassID) override;
+  // Report that the IR was filtered out.
+  void handleFiltered(StringRef PassID, std::string &Name) override;
+  // Report that the pass was ignored.
+  void handleIgnored(StringRef PassID, std::string &Name) override;
+  // Make substitutions in \p S suitable for reporting changes
+  // after the pass and then print it.
+
+  raw_ostream &Out;
+};
+
 // 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 IRChangePrinter : public ChangePrinter<std::string> {
+class IRChangedPrinter : public TextChangeReporter<std::string> {
 public:
-  IRChangePrinter();
-  ~IRChangePrinter() override;
+  IRChangedPrinter() {}
+  ~IRChangedPrinter() override;
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
 protected:
-  // Called on the first IR processed.
-  void handleInitialIR(Any IR) override;
   // 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 the pass is not iteresting.
-  void omitAfter(StringRef PassID, std::string &Name) 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 when an interesting pass is invalidated.
-  void handleInvalidated(StringRef PassID) override;
-  // Called when the IR or pass is not interesting.
-  void handleFiltered(StringRef PassID, std::string &Name) override;
-  // Called when an ignored pass is encountered.
-  void handleIgnored(StringRef PassID, std::string &Name) override;
   // Called to compare the before and after representations of the IR.
   bool same(const std::string &Before, const std::string &After) override;
-
-  raw_ostream &Out;
 };
 
 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;
-  IRChangePrinter PrintChangedIR;
+  IRChangedPrinter PrintChangedIR;
   VerifyInstrumentation Verify;
 
   bool VerifyEach;
 
 public:
   StandardInstrumentations(bool DebugLogging, bool VerifyEach = false)
       : PrintPass(DebugLogging), OptNone(DebugLogging), Verify(DebugLogging),
         VerifyEach(VerifyEach) {}
 
   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

 }
 
 /// Generic IR-printing helper that unpacks a pointer to IRUnit wrapped into
 /// llvm::Any and does actual print job.
 void unwrapAndPrint(raw_ostream &OS, Any IR, StringRef Banner,
                     bool ForceModule = false, bool Brief = false,
                     bool ShouldPreserveUseListOrder = false) {
   if (ForceModule) {
     if (auto UnwrappedModule = unwrapModule(IR))
       printIR(OS, UnwrappedModule->first, Banner, UnwrappedModule->second,
               Brief, ShouldPreserveUseListOrder);
     return;
   }
 
   if (any_isa<const Module *>(IR)) {
     const Module *M = any_cast<const Module *>(IR);
     assert(M && "module should be valid for printing");
     printIR(OS, M, Banner, "", Brief, ShouldPreserveUseListOrder);
     return;
   }
 
   if (any_isa<const Function *>(IR)) {
     const Function *F = any_cast<const Function *>(IR);
     assert(F && "function should be valid for printing");
     printIR(OS, F, Banner, "", Brief);
     return;
   }
 
   if (any_isa<const LazyCallGraph::SCC *>(IR)) {
     const LazyCallGraph::SCC *C = any_cast<const LazyCallGraph::SCC *>(IR);
     assert(C && "scc should be valid for printing");
     std::string Extra = std::string(formatv(" (scc: {0})", C->getName()));
     printIR(OS, C, Banner, Extra, Brief);
     return;
   }
 
   if (any_isa<const Loop *>(IR)) {
     const Loop *L = any_cast<const Loop *>(IR);
     assert(L && "Loop should be valid for printing");
     printIR(OS, L, Banner, Brief);
     return;
   }
   llvm_unreachable("Unknown wrapped IR type");
 }
 
 // Return true when this is a pass for which changes should be ignored
 bool isIgnored(StringRef PassID) {
   return isSpecialPass(PassID,
                        {"PassManager", "PassAdaptor", "AnalysisManagerProxy"});
 }
 
-// Return true when this is a defined function for which printing
-// of changes is desired.
-bool isInterestingFunction(const Function &F) {
+} // namespace
+
+template <typename IRUnitT>
+ChangeReporter<IRUnitT>::~ChangeReporter<IRUnitT>() {
+  assert(BeforeStack.empty() && "Problem with Change Printer stack.");
+}
+
+template <typename IRUnitT>
+bool ChangeReporter<IRUnitT>::isInterestingFunction(const Function &F) {
   return llvm::isFunctionInPrintList(F.getName());
 }
 
-// Return true when this is a pass for which printing of changes is desired.
-bool isInterestingPass(StringRef PassID) {
+template <typename IRUnitT>
+bool ChangeReporter<IRUnitT>::isInterestingPass(StringRef PassID) {
   if (isIgnored(PassID))
     return false;
 
   static std::unordered_set<std::string> PrintPassNames(PrintPassesList.begin(),
                                                         PrintPassesList.end());
   return PrintPassNames.empty() || PrintPassNames.count(PassID.str());
 }
 
 // Return true when this is a pass on IR for which printing
 // of changes is desired.
-bool isInteresting(Any IR, StringRef PassID) {
+template <typename IRUnitT>
+bool ChangeReporter<IRUnitT>::isInteresting(Any IR, StringRef PassID) {
   if (!isInterestingPass(PassID))
     return false;
   if (any_isa<const Function *>(IR))
     return isInterestingFunction(*any_cast<const Function *>(IR));
   return true;
 }
 
-} // namespace
-
 template <typename IRUnitT>
-void ChangePrinter<IRUnitT>::saveIRBeforePass(Any IR, StringRef PassID) {
+void ChangeReporter<IRUnitT>::saveIRBeforePass(Any IR, StringRef PassID) {
   // Always need to place something on the stack because invalidated passes
   // are not given the IR so it cannot be determined whether the pass was for
   // something that was filtered out.
   BeforeStack.emplace_back();
 
   if (!isInteresting(IR, PassID))
     return;
   // Is this the initial IR?
   if (InitialIR) {
     InitialIR = false;
     handleInitialIR(IR);
   }
 
   // Save the IR representation on the stack.
   IRUnitT &Data = BeforeStack.back();
   generateIRRepresentation(IR, PassID, Data);
 }
 
 template <typename IRUnitT>
-void ChangePrinter<IRUnitT>::handleIRAfterPass(Any IR, StringRef PassID) {
+void ChangeReporter<IRUnitT>::handleIRAfterPass(Any IR, StringRef PassID) {
   assert(!BeforeStack.empty() && "Unexpected empty stack encountered.");
   std::string Name;
 
   // unwrapModule has inconsistent handling of names for function IRs.
   if (any_isa<const Function *>(IR)) {
     const Function *F = any_cast<const Function *>(IR);
     Name = formatv(" (function: {0})", F->getName()).str();
   } else {
     if (auto UM = unwrapModule(IR))
       Name = UM->second;
   }
-  if (Name.empty())
+  if (Name == "")
     Name = " (module)";
 
   if (isIgnored(PassID))
     handleIgnored(PassID, Name);
   else if (!isInteresting(IR, PassID))
     handleFiltered(PassID, Name);
   else {
     // Get the before rep from the stack
     IRUnitT &Before = BeforeStack.back();
     // Create the after rep
     IRUnitT After;
     generateIRRepresentation(IR, PassID, After);
 
     // Was there a change in IR?
     if (same(Before, After))
       omitAfter(PassID, Name);
     else
       handleAfter(PassID, Name, Before, After, IR);
   }
   BeforeStack.pop_back();
 }
 
 template <typename IRUnitT>
-void ChangePrinter<IRUnitT>::handleInvalidatedPass(StringRef PassID) {
+void ChangeReporter<IRUnitT>::handleInvalidatedPass(StringRef PassID) {
   assert(!BeforeStack.empty() && "Unexpected empty stack encountered.");
 
   // Always flag it as invalidated as we cannot determine when
   // a pass for a filtered function is invalidated since we do not
   // get the IR in the call.  Also, the output is just alternate
   // forms of the banner anyway.
   handleInvalidated(PassID);
   BeforeStack.pop_back();
 }
 
-template <typename IRUnitT> ChangePrinter<IRUnitT>::~ChangePrinter<IRUnitT>() {
-  assert(BeforeStack.empty() && "Problem with Change Printer stack.");
-}
-
-IRChangePrinter::IRChangePrinter() : Out(dbgs()) {}
-
-IRChangePrinter::~IRChangePrinter() {}
-
-void IRChangePrinter::registerCallbacks(PassInstrumentationCallbacks &PIC) {
-  if (!PrintChanged)
-    return;
-
+template <typename IRUnitT>
+void ChangeReporter<IRUnitT>::registerRequiredCallbacks(
+    PassInstrumentationCallbacks &PIC) {
   PIC.registerBeforeNonSkippedPassCallback(
       [this](StringRef P, Any IR) { saveIRBeforePass(IR, P); });
 
   PIC.registerAfterPassCallback(
       [this](StringRef P, Any IR, const PreservedAnalyses &) {
         handleIRAfterPass(IR, P);
       });
   PIC.registerAfterPassInvalidatedCallback(
       [this](StringRef P, const PreservedAnalyses &) {
         handleInvalidatedPass(P);
       });
 }
 
-void IRChangePrinter::handleInitialIR(Any IR) {
+template <typename IRUnitT>
+TextChangeReporter<IRUnitT>::TextChangeReporter()
+    : ChangeReporter<IRUnitT>(), Out(dbgs()) {}
+
+template <typename IRUnitT>
+void TextChangeReporter<IRUnitT>::handleInitialIR(Any IR) {
   // Always print the module.
   // Unwrap and print directly to avoid filtering problems in general routines.
   auto UnwrappedModule = unwrapModule(IR, /*Force=*/true);
   assert(UnwrappedModule && "Expected module to be unwrapped when forced.");
   Out << "*** IR Dump At Start: ***" << UnwrappedModule->second << "\n";
   UnwrappedModule->first->print(Out, nullptr,
                                 /*ShouldPreserveUseListOrder=*/true);
 }
 
-void IRChangePrinter::generateIRRepresentation(Any IR, StringRef PassID,
+template <typename IRUnitT>
+void TextChangeReporter<IRUnitT>::omitAfter(StringRef PassID,
+                                            std::string &Name) {
+  Out << formatv("*** IR Dump After {0}{1} omitted because no change ***\n",
+                 PassID, Name);
+}
+
+template <typename IRUnitT>
+void TextChangeReporter<IRUnitT>::handleInvalidated(StringRef PassID) {
+  Out << formatv("*** IR Pass {0} invalidated ***\n", PassID);
+}
+
+template <typename IRUnitT>
+void TextChangeReporter<IRUnitT>::handleFiltered(StringRef PassID,
+                                                 std::string &Name) {
+  SmallString<20> Banner =
+      formatv("*** IR Dump After {0}{1} filtered out ***\n", PassID, Name);
+  Out << Banner;
+}
+
+template <typename IRUnitT>
+void TextChangeReporter<IRUnitT>::handleIgnored(StringRef PassID,
+                                                std::string &Name) {
+  Out << formatv("*** IR Pass {0}{1} ignored ***\n", PassID, Name);
+}
+
+IRChangedPrinter::~IRChangedPrinter() {}
+
+void IRChangedPrinter::registerCallbacks(PassInstrumentationCallbacks &PIC) {
+  if (PrintChanged)
+    TextChangeReporter<std::string>::registerRequiredCallbacks(PIC);
+}
+
+void IRChangedPrinter::generateIRRepresentation(Any IR, StringRef PassID,
                                                 std::string &Output) {
   raw_string_ostream OS(Output);
   // use the after banner for all cases so it will match
   SmallString<20> Banner = formatv("*** IR Dump After {0} ***", PassID);
-  unwrapAndPrint(OS, IR, Banner, llvm::forcePrintModuleIR(),
+  unwrapAndPrint(OS, IR, Banner, forcePrintModuleIR(),
                  /*Brief=*/false, /*ShouldPreserveUseListOrder=*/true);
-  OS.str();
-}
 
-void IRChangePrinter::omitAfter(StringRef PassID, std::string &Name) {
-  Out << formatv("*** IR Dump After {0}{1} omitted because no change ***\n",
-                 PassID, Name);
+  OS.str();
 }
 
-void IRChangePrinter::handleAfter(StringRef PassID, std::string &Name,
+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) {
     Out << "*** IR Dump Before" << Banner.substr(17);
void IRChangedPrinter::handleAfter(StringRef PassID, std::string &Name,
   // 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());
 }
 
-void IRChangePrinter::handleInvalidated(StringRef PassID) {
-  Out << formatv("*** IR Pass {0} invalidated ***\n", PassID);
-}
-
-void IRChangePrinter::handleFiltered(StringRef PassID, std::string &Name) {
-  SmallString<20> Banner =
-      formatv("*** IR Dump After {0}{1} filtered out ***\n", PassID, Name);
-  Out << Banner;
-}
-
-void IRChangePrinter::handleIgnored(StringRef PassID, std::string &Name) {
-  Out << formatv("*** IR Pass {0}{1} ignored ***\n", PassID, Name);
-}
-
-bool IRChangePrinter::same(const std::string &Before,
-                           const std::string &After) {
-  return Before == After;
+bool IRChangedPrinter::same(const std::string &S1, const std::string &S2) {
+  return S1 == S2;
 }
 
 PrintIRInstrumentation::~PrintIRInstrumentation() {
   assert(ModuleDescStack.empty() && "ModuleDescStack is not empty at exit");
 }
 
 void PrintIRInstrumentation::pushModuleDesc(StringRef PassID, Any IR) {
   assert(StoreModuleDesc);
void PrintIRInstrumentation::printBeforePass(StringRef PassID, Any IR) {
   if (PassID.startswith("PassManager<") || PassID.contains("PassAdaptor<"))
     return;
 
   // Saving Module for AfterPassInvalidated operations.
   // Note: here we rely on a fact that we do not change modules while
   // traversing the pipeline, so the latest captured module is good
   // for all print operations that has not happen yet.
   if (StoreModuleDesc && llvm::shouldPrintAfterPass(PassID))
     pushModuleDesc(PassID, IR);
 
   if (!llvm::shouldPrintBeforePass(PassID))
     return;
 
   SmallString<20> Banner = formatv("*** IR Dump Before {0} ***", PassID);
   unwrapAndPrint(dbgs(), IR, Banner, llvm::forcePrintModuleIR());
   return;
 }
void StandardInstrumentations::registerCallbacks(
   TimePasses.registerCallbacks(PIC);
   OptNone.registerCallbacks(PIC);
   OptBisect.registerCallbacks(PIC);
   PreservedCFGChecker.registerCallbacks(PIC);
   PrintChangedIR.registerCallbacks(PIC);
   if (VerifyEach)
     Verify.registerCallbacks(PIC);
 }
+
+namespace llvm {
+
+template class ChangeReporter<std::string>;
+template class TextChangeReporter<std::string>;
+
+} // namespace llvm