Introduce -print-changed=[diff | diff-quiet] which show changes in patch-like format

[jamieschmeiser]

Summary

Introduce -print-changed=[diff | diff-quiet] which print the changes made by a pass in patch-like form.

Introduce base classes that hold a textual represent of the IR
based on basic blocks and a base class for comparing this
representation.  A new change printer is introduced that uses these
classes to save and compare representations of the IR before and after
each pass.  It only reports when changes are made by a pass (similar to
-print-changed) except that the changes are shown in a patch-like format
with those lines that are removed shown in red prefixed with '-' and those
added shown in green with '+'.  This functionality was introduced in my
tutorial at the 2020 virtual developer's meeting.

[jamieschmeiser]

Changes based on clang-tidy/format

[aeubanks]

llvm/include/llvm/Passes/StandardInstrumentations.h

274

Apologies if I'm forgetting things you've said in the past, but what are the other future use cases of this? I'd prefer less templatization when possible.

llvm/lib/Passes/StandardInstrumentations.cpp

96

what about "print-changed-diff"? "print-changes" seems a bit too closed to "print-changed"

109

probably shouldn't have Linux in the name, this could work on other platforms.

145

maybe this should be using the APIs in Program.h instead? popen seems a bit too C-like.

llvm/test/Other/ChangePrinters/lit.local.cfg

14–17

return '-line-format' in ld_out

[aeubanks]

Also, could you say in the description that this shells out to diff?

[jamieschmeiser]

The template base classes are also used for -dot-cfg-changes, which shows the diff by making a website and showing the changes in dot-cfg form with the added code shown in green and the deleted code shown in red. The template parameters are the data structures needed for that display, while they are just empty structs in this change reporter. https://reviews.llvm.org/D87202 has the changes for that change reporter but the changes are out-of-date as they build on the original patch for this change reporter (which has been split up), but the data structures used by the templates for that change reporter can be seen in that patch.

I'm fine with changing the option name. I will update the patch and address the other comments.

[jamieschmeiser]

Respond to review comments: change option name to print-changed-diff,
change function name and change code to use ExecuteAndWait.

Also add hidden option specifying fully resolved path name to diff to
allow specifying overrides, defaulting to typical linux system path.

[aeubanks]

There are a lot of unnecessary llvm::, can those be removed?

TBH this IRComparer framework seems over-engineered and it's hard for me to understand all the moving parts and how clients override certain methods, making it hard to review.
Is it possible to make it simpler?

The diff part looks mostly good though.

llvm/include/llvm/Passes/StandardInstrumentations.h

252

this part is unnecessary

277

is a separate method necessary? or can it be done in the constructor?

331

348

can you use an enum class? otherwise it'll pollute the llvm namespace

362–363

These don't match the names below.
Also would it be simpler to just allow each subclass to handle Function vs Module, rather than having virtual methods for each one?

366

the return value is never used

llvm/lib/Passes/StandardInstrumentations.cpp

100

DiffBinary or DiffBinaryPath?

101

should this default to the diff on the PATH?

448

does this work?

474

looks like StringMap already has an operator==, why recreate it here?

[jamieschmeiser]

I have removed the templates and the compare hierarchy
because they are not necessary for this change reporter.
They will have to be re-introduced for the final reporter as
it builds on this one but their removal simplifies this review.

I have addressed your other comments and also made
changes so that the differences are shown in red and
green in addition to being prefixed with '-' and '+', respectively.

[jamieschmeiser]

I would like to change the option handling for this based on the discussions in https://reviews.llvm.org/D92589. In particular, I was thinking that this option could be controlled using -print-changed=diff and -print-changed=diff-quiet rather than using a separate option. WDYT? I will wait until that patch is settled before making further changes to this one.

[aeubanks]

In D91890#2463537, @jamieschmeiser wrote:

I would like to change the option handling for this based on the discussions in https://reviews.llvm.org/D92589. In particular, I was thinking that this option could be controlled using -print-changed=diff and -print-changed=diff-quiet rather than using a separate option. WDYT? I will wait until that patch is settled before making further changes to this one.

That makes a lot of sense to me.

[jamieschmeiser]

Updated option control to be -print-changed=[diff | diff-quiet]

rather than a separate option.  This also introduces the
quiet mode.  Change/add tests accordingly.

[aeubanks]

just a general comment, I do like the idea of printing diffs between passes, could make looking through pipelines much nicer

llvm/include/llvm/Passes/StandardInstrumentations.h

277

seems weird to inherit from StringMap, what about making it a variable instead?

290

looks like this is only used in the cpp file, move it there?

llvm/lib/Passes/StandardInstrumentations.cpp

103

diff

106

something containing the word "path" or "binary" seems clearer, like print-changed-diff-binary or print-changed-diff-path

116–117

If the temp files are removed at the end, is there any reason for these to be static? Can we just create new temp files every time?

474

ping
(and if you make ChangedFuncData contain a StringMap as a variable you can just compare the two)

621

I thought we already had similar utilities for this.
Should probably be a static function instead of part of ChangedIRComparer.

626

C->begin()->getFunction().getParent(). SCCs cannot be empty.

1041

maybe we can just assume that ChangedIRData has an equality operator? not necessarily in this patch

1057

this won't necessarily be in the same order that the blocks are laid out in the function right? that seems like something we should make sure happens

[jamieschmeiser]

Respond to review comments:
Establish an ordering based on the "after" ordering with the removed
sections interspersed. This ordering is applied to get the basic blocks
in the order they are in the function and also applied to the functions
to get them into the same order they are in the module.

[aeubanks]

mostly looks good, a couple nits

llvm/include/llvm/Passes/StandardInstrumentations.h

295

300

function_ref is much lighter weight.

llvm/lib/Passes/StandardInstrumentations.cpp

67–68

The wording here tripped me up, perhaps The values "diff" and "diff-quiet" will....

76

enum class?

587

1025–1072

can you move these above the StandardInstrumentations code?

llvm/test/Other/ChangePrinters/lit.local.cfg

6

I'm wondering if we should just detect any diff in the PATH, but maybe just /usr/bin/diff is fine, idk

[jamieschmeiser]

Respond to review comments. Minor changes including fixing typos, using function_ref, enum classes and moving code around.

llvm/include/llvm/Passes/StandardInstrumentations.h

protected:
   // 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;
 };
 
+// The following classes hold a representation of the IR for a change
+// reporter that uses string comparisons of the basic blocks
+// that are created using print (ie, similar to dump()).
+// These classes respect the filtering of passes and functions using
+// -filter-passes and -filter-print-funcs.
+//
+// Information that needs to be saved for a basic block in order to compare
+// before and after the pass to determine if it was changed by a pass.
+class ChangedBlockData {
+public:
+  ChangedBlockData(const BasicBlock &B);
+
+  bool operator!=(const ChangedBlockData &That) const {
+    return Body != That.Body;
+  }
+
+  // Return the label of the represented basic block.
+  StringRef getLabel() const { return Label; }
+  // Return the string representation of the basic block.
+  StringRef getBody() const { return Body; }
+
+protected:
+  std::string Label;
+  std::string Body;
+};
+
+// The data saved for comparing functions.
+class ChangedFuncData : public StringMap<ChangedBlockData> {
+public:
+  ChangedFuncData() {}
+  ChangedFuncData(const Function &F);
+  bool operator==(const ChangedFuncData &That) const;
+  bool operator!=(const ChangedFuncData &That) const {
+    return !(*this == That);
+  }
+};
+
+// A map of names to the saved data.
+using ChangedIRData = StringMap<ChangedFuncData>;
+
+enum class IRChangeDiffType { InBefore, InAfter, IsCommon };
+
+// A class that compares two IRs and does a diff between them.  The
+// added lines are prefixed with a '+', the removed lines are prefixed
+// with a '-' and unchanged lines are prefixed with a space (to have
+// things line up).
+class ChangedIRComparer {
+public:
+  ChangedIRComparer(raw_ostream &OS, const ChangedIRData &Before,
+                    const ChangedIRData &After)
+      : Before(Before), After(After), Out(OS) {}
+
+  // Compare the 2 IRs.
+  void compare(Any IR, StringRef Prefix, StringRef PassID, StringRef Name);
+
+  // Analyze \p IR and build the IR representation in \p Data.
+  static void analyzeIR(Any IR, ChangedIRData &Data);
+
+protected:
+  // Return the module when that is the appropriate level of
+  // comparison for \p IR.
+  static const Module *getModule(Any IR);
+
+  // Generate the data for \p F into \p Data.
+  static bool generateFunctionData(ChangedIRData &Data, const Function &F);
+
+  // Called to handle the compare of a function. When \p InModule is set,
+  // this function is being handled as part of comparing a module.
+  void handleFunctionCompare(StringRef Name, StringRef Prefix, StringRef PassID,
+                             bool InModule, const ChangedFuncData &Before,
+                             const ChangedFuncData &After);
+
+  const ChangedIRData &Before;
+  const ChangedIRData &After;
+  raw_ostream &Out;
+};
+
+// A change printer that prints out in-line differences in the basic
+// blocks.  It uses an InlineComparer to do the comparison so it shows
+// the differences prefixed with '-' and '+' for code that is removed
+// and added, respectively.  Changes to the IR that do not affect basic
+// blocks are not reported as having changed the IR.  The option
+// -print-module-scope does not affect this change reporter.
+class InLineChangePrinter : public TextChangeReporter<ChangedIRData> {
+public:
+  InLineChangePrinter() {}
+  ~InLineChangePrinter() override;
+  void registerCallbacks(PassInstrumentationCallbacks &PIC);
+
+protected:
+  // Create a representation of the IR.
+  virtual void generateIRRepresentation(Any IR, StringRef PassID,
+                                        ChangedIRData &Output) override;
+
+  // Called when an interesting IR has changed.
+  virtual void handleAfter(StringRef PassID, std::string &Name,
+                           const ChangedIRData &Before,
+                           const ChangedIRData &After, Any) override;
+  // Called to compare the before and after representations of the IR.
+  virtual bool same(const ChangedIRData &Before,
+                    const ChangedIRData &After) 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;
+  InLineChangePrinter PrintChangedDiff;
   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>;
 
+extern template class ChangeReporter<ChangedIRData>;
+extern template class TextChangeReporter<ChangedIRData>;
+
 } // namespace llvm
 
 #endif

llvm/lib/Passes/StandardInstrumentations.cpp

 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include <unordered_set>
 #include <vector>
 
 using namespace llvm;
 
 cl::opt<bool> PreservedCFGCheckerInstrumentation::VerifyPreservedCFG(
     "verify-cfg-preserved", cl::Hidden,

 // 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.  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);
+// An option -print-changed-diff which prints in line differences of
+// changed IR as they progress through the pipeline.  The differences
+// are presented in a form similar to a patch.  The lines that are removed
+// and added are prefixed with '-' and '+', respectively.  The
+// -filter-print-funcs and -filter-passes can be used to filter the output.
+// This reporter relies on the linux diff utility to do comparisons and
+// insert the prefixes.  For systems that do not have the necessary
+// facilities, the error message will be shown in place of the expected output.
+static cl::opt<bool> PrintChangedDiff(
+    "print-changed-diff", cl::Hidden, cl::init(false),
+    cl::desc("Print in-line differences of changes made by a pass"));
+// An option for specifying the diff used by print-changed-diff
+static cl::opt<std::string>
+    DiffBinary("diff-for-print-changed", cl::Hidden, cl::init("diff"),
+               cl::desc("system diff used by change reporters"));
 
 namespace {
 
+// Perform a system based diff between \p Before and \p After, using
+// \p OldLineFormat, \p NewLineFormat, and \p UnchangedLineFormat
+// to control the formatting of the output.  Return an error message
+// for any failures instead of the diff.
+std::string doSystemDiff(StringRef Before, StringRef After,
+                         StringRef OldLineFormat, StringRef NewLineFormat,
+                         StringRef UnchangedLineFormat) {
+  StringRef SR[2]{Before, After};
+  // Store the 2 bodies into temporary files and call diff on them
+  // to get the body of the node.
+  static const unsigned NumFiles = 3;
+  static std::string FileName[NumFiles];
+  static int FD[NumFiles]{-1, -1, -1};
+  for (unsigned I = 0; I < NumFiles; ++I) {
+    if (FD[I] == -1) {
+      SmallVector<char, 200> SV;
+      std::error_code EC =
+          sys::fs::createTemporaryFile("tmpdiff", "txt", FD[I], SV);
+      if (EC)
+        return "Unable to create temporary file.";
+      FileName[I] = Twine(SV).str();
+    }
+    // The third file is used as the result of the diff.
+    if (I == NumFiles - 1)
+      break;
+
+    std::error_code EC = sys::fs::openFileForWrite(FileName[I], FD[I]);
+    if (EC)
+      return "Unable to open temporary file for writing.";
+
+    raw_fd_ostream OutStream(FD[I], /*shouldClose=*/true);
+    if (FD[I] == -1)
+      return "Error opening file for writing.";
+    OutStream << SR[I];
+  }
+
+  static ErrorOr<std::string> DiffExe = sys::findProgramByName(DiffBinary);
+  if (!DiffExe)
+    return "Unable to find diff executable.";
+
+  SmallString<128> OLF = formatv("--old-line-format={0}", OldLineFormat);
+  SmallString<128> NLF = formatv("--new-line-format={0}", NewLineFormat);
+  SmallString<128> ULF =
+      formatv("--unchanged-line-format={0}", UnchangedLineFormat);
+
+  StringRef Args[] = {"-w", "-d", OLF, NLF, ULF, FileName[0], FileName[1]};
+  Optional<StringRef> Redirects[] = {None, StringRef(FileName[2]), None};
+  int Result = sys::ExecuteAndWait(*DiffExe, Args, None, Redirects);
+  if (Result < 0)
+    return "Error executing system diff.";
+  std::string Diff;
+  auto B = MemoryBuffer::getFile(FileName[2]);
+  if (B && *B)
+    Diff = (*B)->getBuffer().str();
+  else
+    return "Unable to read result.";
+
+  // Clean up.
+  for (unsigned I = 0; I < NumFiles; ++I) {
+    std::error_code EC = sys::fs::remove(FileName[I]);
+    if (EC)
+      return "Unable to remove temporary file.";
+  }
+  return Diff;
+}
+
 /// 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))
     return std::make_pair(any_cast<const Module *>(IR), std::string());
 
   if (any_isa<const Function *>(IR)) {

 
 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 isFunctionInPrintList(F.getName());
 }
 
 template <typename IRUnitT>
 bool ChangeReporter<IRUnitT>::isInterestingPass(StringRef PassID) {
   if (isIgnored(PassID))
     return false;
 
   static std::unordered_set<std::string> PrintPassNames(PrintPassesList.begin(),
PIC.registerAfterPassCallback(
         handleIRAfterPass(IR, P);
       });
   PIC.registerAfterPassInvalidatedCallback(
       [this](StringRef P, const PreservedAnalyses &) {
         handleInvalidatedPass(P);
       });
 }
 
+ChangedBlockData::ChangedBlockData(const BasicBlock &B)
+    : Label(B.getName().str()) {
+  raw_string_ostream SS(Body);
+  B.print(SS, nullptr, true, true);
+}
+
+ChangedFuncData::ChangedFuncData(const Function &F) {
+  for (const auto &B : F)
+    insert({B.getName(), B});
+}
+
+bool ChangedFuncData::operator==(const ChangedFuncData &That) const {
+  for (auto &B : That) {
+    StringRef Label = B.getKey();
+    if (count(Label) == 0)
+      return false;
+    const ChangedBlockData &ThisData = find(Label)->getValue();
+    const ChangedBlockData &ThatData = B.getValue();
+    if (ThisData != ThatData)
+      return false;
+  }
+  return true;
+}
+
 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 IRChangedPrinter::handleAfter(StringRef PassID, std::string &Name,
   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())
+    if (Name.substr(0, 6) != " (scc:" && !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())
+  if (Name.substr(0, 6) != " (scc:" && !forcePrintModuleIR())
     Out << Name;
 
   Out << After.substr(Banner.size());
 }
 
 bool IRChangedPrinter::same(const std::string &S1, const std::string &S2) {
   return S1 == S2;
 }
 
+void ChangedIRComparer::compare(Any IR, StringRef Prefix, StringRef PassID,
+                                StringRef Name) {
+  if (!getModule(IR)) {
+    // Not a module so just handle the single function.
+    assert(Before.size() == 1 && "Expected only one function.");
+    assert(After.size() == 1 && "Expected only one function.");
+    handleFunctionCompare(Name, Prefix, PassID, false,
+                          Before.begin()->getValue(),
+                          After.begin()->getValue());
+    return;
+  }
+  // Determine whether functions are common or not.
+  StringMap<IRChangeDiffType> Funcs;
+  for (auto &B : Before)
+    Funcs.insert({B.getKey(), IRChangeDiffType::InBefore});
+  for (auto &A : After)
+    if (Funcs.count(A.getKey()) == 1)
+      Funcs[A.getKey()] = IRChangeDiffType::IsCommon;
+    else
+      Funcs.insert({A.getKey(), IRChangeDiffType::InAfter});
+
+  for (auto &F : Funcs) {
+    Name = F.getKey();
+    IRChangeDiffType CDDT = F.getValue();
+    // If the function isn't common, create an empty version for the compare.
+    if (CDDT == IRChangeDiffType::InBefore)
+      handleFunctionCompare(Name, Prefix, PassID, true,
+                            Before.find(Name)->getValue(), ChangedFuncData());
+    else if (CDDT == IRChangeDiffType::InAfter)
+      handleFunctionCompare(Name, Prefix, PassID, true, ChangedFuncData(),
+                            After.find(Name)->getValue());
+    else {
+      assert(CDDT == IRChangeDiffType::IsCommon && "Unexpected Diff type.");
+      handleFunctionCompare(Name, Prefix, PassID, true,
+                            Before.find(Name)->getValue(),
+                            After.find(Name)->getValue());
+    }
+  }
+}
+
+void ChangedIRComparer::analyzeIR(Any IR, ChangedIRData &Data) {
+  if (const Module *M = getModule(IR)) {
+    // Create data for each existing/interesting function in the module.
+    for (const Function &F : *M)
+      generateFunctionData(Data, F);
+    return;
+  }
+
+  const Function *F = nullptr;
+  if (any_isa<const Function *>(IR))
+    F = any_cast<const Function *>(IR);
+  else {
+    assert(any_isa<const Loop *>(IR) && "Unknown IR unit.");
+    const Loop *L = any_cast<const Loop *>(IR);
+    F = L->getHeader()->getParent();
+  }
+  assert(F && "Unknown IR unit.");
+  generateFunctionData(Data, *F);
+}
+
+const Module *ChangedIRComparer::getModule(Any IR) {
+  if (any_isa<const Module *>(IR))
+    return any_cast<const Module *>(IR);
+  if (any_isa<const LazyCallGraph::SCC *>(IR)) {
+    const LazyCallGraph::SCC *C = any_cast<const LazyCallGraph::SCC *>(IR);
+    for (const LazyCallGraph::Node &N : *C)
+      return N.getFunction().getParent();
+  }
+  return nullptr;
+}
+
+bool ChangedIRComparer::generateFunctionData(ChangedIRData &Data,
+                                             const Function &F) {
+  if (!F.isDeclaration() && isFunctionInPrintList(F.getName())) {
+    Data.insert({F.getName(), F});
+    return true;
+  }
+  return false;
+}
+
 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!");
         }
       });
 }
 
+InLineChangePrinter::~InLineChangePrinter() {}
+
+void InLineChangePrinter::generateIRRepresentation(Any IR, StringRef PassID,
+                                                   ChangedIRData &D) {
+  ChangedIRComparer::analyzeIR(IR, D);
+}
+
+void InLineChangePrinter::handleAfter(StringRef PassID, std::string &Name,
+                                      const ChangedIRData &Before,
+                                      const ChangedIRData &After, Any IR) {
+  if (Name == "")
+    Name = " (module)";
+  SmallString<20> Banner =
+      formatv("*** IR Dump After {0} ***{1}\n", PassID, Name);
+  Out << Banner;
+  ChangedIRComparer(Out, Before, After).compare(IR, "", PassID, Name);
+  Out << "\n";
+}
+
+bool InLineChangePrinter::same(const ChangedIRData &D1,
+                               const ChangedIRData &D2) {
+  return D1 == D2;
+}
+
+void ChangedIRComparer::handleFunctionCompare(StringRef Name, StringRef Prefix,
+                                              StringRef PassID, bool InModule,
+                                              const ChangedFuncData &Before,
+                                              const ChangedFuncData &After) {
+  // Print a banner when this is being shown in the context of a module
+  if (InModule)
+    Out << "\n*** IR for function " << Name << " ***\n";
+
+  const std::string Removed = "\033[31m-%l\033[0m\n";
+  const std::string Added = "\033[32m+%l\033[0m\n";
+  const std::string NoChange = " %l\n";
+  for (const auto &B : Before) {
+    const auto &BeforeBD = B.getValue();
+    if (After.count(BeforeBD.getLabel())) {
+      // This block is in both
+      const auto &BD = After.find(BeforeBD.getLabel())->getValue();
+      Out << doSystemDiff(BeforeBD.getBody(), BD.getBody(), Removed, Added,
+                          NoChange);
+    } else
+      // This has been removed
+      Out << doSystemDiff(BeforeBD.getBody(), "\n", Removed, Added, NoChange);
+  }
+  for (const auto &B : After) {
+    const auto &AfterBD = B.getValue();
+    if (!Before.count(AfterBD.getLabel())) {
+      // This block is new
+      Out << doSystemDiff("\n", AfterBD.getBody(), Removed, Added, NoChange);
+    }
+  }
+}
+
+void InLineChangePrinter::registerCallbacks(PassInstrumentationCallbacks &PIC) {
+  if (PrintChangedDiff)
+    TextChangeReporter<ChangedIRData>::registerRequiredCallbacks(PIC);
+}
+
 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);
   if (VerifyEach)
     Verify.registerCallbacks(PIC);
+  PrintChangedDiff.registerCallbacks(PIC);
 }
 
 namespace llvm {
 
 template class ChangeReporter<std::string>;
 template class TextChangeReporter<std::string>;
 
+template class ChangeReporter<ChangedIRData>;
+template class TextChangeReporter<ChangedIRData>;
+
 } // namespace llvm

llvm/test/Other/ChangePrinters/lit.local.cfg

+import os
+import subprocess
+
+def have_needed_diff_support():
+    if not os.path.exists('/usr/bin/diff'):
+        return False
+
+    ld_cmd = subprocess.Popen(
+        ['/usr/bin/diff', '--help'], stdout=subprocess.PIPE, env={'LANG': 'C'})
+    ld_out = ld_cmd.stdout.read().decode()
+    ld_cmd.wait()
+
+    return '-line-format' in ld_out
+
+if not have_needed_diff_support():
+  config.unsupported = True

llvm/test/Other/ChangePrinters/print-changes.ll

+; Simple checks of -print-changed-diff
+;
+; Note that (mostly) only the banners are checked.
+;
+; Simple functionality check.
+; RUN: opt -S -print-changed-diff -passes=instsimplify 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-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-diff -passes=instsimplify -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FUNC-FILTER
+;
+; Check that the reporting of IRs respects is not affected by
+; -print-module-scope
+; RUN: opt -S -print-changed-diff -passes=instsimplify -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-PRINT-MOD-SCOPE
+;
+; Check that reporting of multiple functions happens
+; RUN: opt -S -print-changed-diff -passes=instsimplify -filter-print-funcs="f,g" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-MULT-FUNC
+;
+; Check that the reporting of IRs respects -filter-passes
+; RUN: opt -S -print-changed-diff -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-PASSES
+;
+; Check that the reporting of IRs respects -filter-passes with multiple passes
+; RUN: opt -S -print-changed-diff -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-MULT-PASSES
+;
+; Check that the reporting of IRs respects both -filter-passes and -filter-print-funcs
+; RUN: opt -S -print-changed-diff -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" -filter-print-funcs=f 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-FUNC-PASSES
+;
+; Check that repeated passes that change the IR are printed and that the
+; others (including g) are filtered out.  Note that only the first time
+; instsimplify is run on f will result in changes
+; RUN: opt -S -print-changed-diff -passes="instsimplify,instsimplify" -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-MULT-PASSES-FILTER-FUNC
+
+define i32 @g() {
+entry:
+  %a = add i32 2, 3
+  ret i32 %a
+}
+
+define i32 @f() {
+entry:
+  %a = add i32 2, 3
+  ret i32 %a
+}
+
+; CHECK-SIMPLE: *** IR Dump At Start: ***
+; CHECK-SIMPLE: ModuleID = {{.+}}
+; CHECK-SIMPLE: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-SIMPLE: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-SIMPLE-NOT: ModuleID = {{.+}}
+; CHECK-SIMPLE-NOT: *** IR{{.*}}
+; CHECK-SIMPLE: entry:
+; CHECK-SIMPLE-NEXT:-  %a = add i32 2, 3
+; CHECK-SIMPLE-NEXT:-  ret i32 %a
+; CHECK-SIMPLE-NEXT:+  ret i32 5
+; CHECK-SIMPLE: *** IR Pass PassManager{{.*}} (function: g) ignored ***
+; CHECK-SIMPLE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-SIMPLE-NOT: ModuleID = {{.+}}
+; CHECK-SIMPLE-NOT: *** IR{{.*}}
+; CHECK-SIMPLE: entry:
+; CHECK-SIMPLE-NEXT:-  %a = add i32 2, 3
+; CHECK-SIMPLE-NEXT:-  ret i32 %a
+; CHECK-SIMPLE-NEXT:+  ret i32 5
+; CHECK-SIMPLE: *** IR Pass PassManager{{.*}} (function: f) ignored ***
+; CHECK-SIMPLE: *** IR Pass ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> (module) ignored ***
+; CHECK-SIMPLE: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-SIMPLE: *** IR Dump After PrintModulePass (module) omitted because no change ***
+
+; CHECK-FUNC-FILTER: *** IR Dump At Start: ***
+; CHECK-FUNC-FILTER-NEXT: ; ModuleID = {{.+}}
+; CHECK-FUNC-FILTER: *** IR Dump After InstSimplifyPass (function: g) filtered out ***
+; CHECK-FUNC-FILTER: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-FUNC-FILTER-NOT: ModuleID = {{.+}}
+; CHECK-FUNC-FILTER: entry:
+; CHECK-FUNC-FILTER:-  %a = add i32 2, 3
+; CHECK-FUNC-FILTER:-  ret i32 %a
+; CHECK-FUNC-FILTER:+  ret i32 5
+; CHECK-FUNC-FILTER: *** IR Pass PassManager{{.*}} (function: f) ignored ***
+; CHECK-FUNC-FILTER: *** IR Pass ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> (module) ignored ***
+; CHECK-FUNC-FILTER: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-FUNC-FILTER: *** IR Dump After PrintModulePass (module) omitted because no change ***
+
+; CHECK-PRINT-MOD-SCOPE: *** IR Dump At Start: ***
+; CHECK-PRINT-MOD-SCOPE: ModuleID = {{.+}}
+; CHECK-PRINT-MOD-SCOPE: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-PRINT-MOD-SCOPE-NOT: ModuleID = {{.+}}
+; CHECK-PRINT-MOD-SCOPE: entry:
+; CHECK-PRINT-MOD-SCOPE:-  %a = add i32 2, 3
+; CHECK-PRINT-MOD-SCOPE:-  ret i32 %a
+; CHECK-PRINT-MOD-SCOPE:+  ret i32 5
+; CHECK-PRINT-MOD-SCOPE: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-PRINT-MOD-SCOPE-NOT: ModuleID = {{.+}}
+; CHECK-PRINT-MOD-SCOPE: entry:
+; CHECK-PRINT-MOD-SCOPE:-  %a = add i32 2, 3
+; CHECK-PRINT-MOD-SCOPE:-  ret i32 %a
+; CHECK-PRINT-MOD-SCOPE:+  ret i32 5
+; CHECK-PRINT-MOD-SCOPE: *** IR Pass PassManager{{.*}} (function: f) ignored ***
+; CHECK-PRINT-MOD-SCOPE: *** IR Pass ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> (module) ignored ***
+; CHECK-PRINT-MOD-SCOPE: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-PRINT-MOD-SCOPE: *** IR Dump After PrintModulePass (module) omitted because no change ***
+
+; CHECK-FILTER-MULT-FUNC: *** IR Dump At Start: ***
+; CHECK-FILTER-MULT-FUNC: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-FILTER-MULT-FUNC-NOT: ModuleID = {{.+}}
+; CHECK-FILTER-MULT-FUNC: entry:
+; CHECK-FILTER-MULT-FUNC:-  %a = add i32 2, 3
+; CHECK-FILTER-MULT-FUNC:-  ret i32 %a
+; CHECK-FILTER-MULT-FUNC:+  ret i32 5
+; CHECK-FILTER-MULT-FUNC: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-FILTER-MULT-FUNC-NOT: ModuleID = {{.+}}
+; CHECK-FILTER-MULT-FUNC: entry:
+; CHECK-FILTER-MULT-FUNC:-  %a = add i32 2, 3
+; CHECK-FILTER-MULT-FUNC:-  ret i32 %a
+; CHECK-FILTER-MULT-FUNC:+  ret i32 5
+; CHECK-FILTER-MULT-FUNC: *** IR Pass PassManager{{.*}} (function: f) ignored ***
+; CHECK-FILTER-MULT-FUNC: *** IR Pass ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> (module) ignored ***
+; CHECK-FILTER-MULT-FUNC: *** IR Dump After VerifierPass (module) omitted because no change ***
+; CHECK-FILTER-MULT-FUNC: *** IR Dump After PrintModulePass (module) omitted because no change ***
+
+; CHECK-FILTER-PASSES: *** IR Dump After InstSimplifyPass (function: g) filtered out ***
+; CHECK-FILTER-PASSES: *** IR Dump At Start: *** (function: g)
+; CHECK-FILTER-PASSES: *** IR Dump After NoOpFunctionPass (function: g) omitted because no change ***
+; CHECK-FILTER-PASSES: *** IR Dump After InstSimplifyPass (function: f) filtered out ***
+; CHECK-FILTER-PASSES: *** IR Dump After NoOpFunctionPass (function: f) omitted because no change ***
+
+; CHECK-FILTER-MULT-PASSES: *** IR Dump At Start: *** (function: g)
+; CHECK-FILTER-MULT-PASSES: *** IR Dump After InstSimplifyPass *** (function: g)
+; CHECK-FILTER-MULT-PASSES-NOT: ModuleID = {{.+}}
+; CHECK-FILTER-MULT-PASSES: entry:
+; CHECK-FILTER-MULT-PASSES:-  %a = add i32 2, 3
+; CHECK-FILTER-MULT-PASSES:-  ret i32 %a
+; CHECK-FILTER-MULT-PASSES:+  ret i32 5
+; CHECK-FILTER-MULT-PASSES: *** IR Dump After NoOpFunctionPass (function: g) omitted because no change ***
+; CHECK-FILTER-MULT-PASSES: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-FILTER-MULT-PASSES-NOT: ModuleID = {{.+}}
+; CHECK-FILTER-MULT-PASSES: entry:
+; CHECK-FILTER-MULT-PASSES:-  %a = add i32 2, 3
+; CHECK-FILTER-MULT-PASSES:-  ret i32 %a
+; CHECK-FILTER-MULT-PASSES:+  ret i32 5
+; CHECK-FILTER-MULT-PASSES: *** IR Dump After NoOpFunctionPass (function: f) omitted because no change ***
+
+; CHECK-FILTER-FUNC-PASSES: *** IR Dump After InstSimplifyPass (function: g) filtered out ***
+; CHECK-FILTER-FUNC-PASSES: *** IR Dump After NoOpFunctionPass (function: g) filtered out ***
+; CHECK-FILTER-FUNC-PASSES: *** IR Dump At Start: *** (function: f)
+; CHECK-FILTER-FUNC-PASSES: *** IR Dump After InstSimplifyPass *** (function: f)
+; CHECK-FILTER-FUNC-PASSES-NOT: ModuleID = {{.+}}
+; CHECK-FILTER-FUNC-PASSES: entry:
+; CHECK-FILTER-FUNC-PASSES:-  %a = add i32 2, 3
+; CHECK-FILTER-FUNC-PASSES:-  ret i32 %a
+; CHECK-FILTER-FUNC-PASSES:+  ret i32 5
+; CHECK-FILTER-FUNC-PASSES: *** IR Dump After NoOpFunctionPass (function: f) omitted because no change ***
+
+; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump At Start: ***
+; 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-NOT: ModuleID = {{.+}}
+; CHECK-MULT-PASSES-FILTER-FUNC: entry:
+; CHECK-MULT-PASSES-FILTER-FUNC:-  %a = add i32 2, 3
+; CHECK-MULT-PASSES-FILTER-FUNC:-  ret i32 %a
+; CHECK-MULT-PASSES-FILTER-FUNC:+  ret i32 5
+; CHECK-MULT-PASSES-FILTER-FUNC: *** IR Dump After InstSimplifyPass (function: f) omitted because no change ***