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"

110

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

146

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?

446

does this work?

472

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

104

diff

107

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

117–118

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?

472

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

636

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

641

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

1081

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

1097

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?

602

1065–1112

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 abstract template base classes contain the necessary code to
+// create a change reporter that uses string comparisons of the basic blocks
+// that are created using Value::print (ie, similar to dump()).
+// These classes allow one to associate extra data with each basic block
+// and the structs representing the basic blocks are grouped into
+// the structs representing the functions.  Essentially, overrides
+// in the classes derived from ChangeReporter use a class derived from
+// IRComparer to compare representations of IRs.  The overrides are called
+// when changes are detected after each pass to generate the output while the
+// mechanics of detecting changes is handled in the base classes.  The
+// saving of extra data that may be desired by the overrides is done
+// in a class derived from IRComparer, as is the actual generation
+// of the output in response to changes to the IR.
+// These change reporters work on basic blocks as the basic unit, meaning
+// that comparisons between the before and after are made by comparing
+// basic blocks and showing the differences between basic blocks.  Basic
+// blocks are matched from comparison using the name of the basic block
+// so if a basic block is renamed, it will be considered to be deleted
+// and the renamed block as added since the names do not match.
+// 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.
+template <typename BlockDataType> class BlockDataTemplate {
+public:
+  // Note that the constructor will call a static function
+  // BlockDataType::initialize(BlockDataType &, BasicBlock &)
+  // to fill in the Data member.
+  BlockDataTemplate(const BasicBlock &B);
+
+  bool operator!=(const BlockDataTemplate &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; }
+
+  // Return the associated data
+  const BlockDataType &getData() const { return Data; }
+
+protected:
+  std::string Label;
+  std::string Body;
+
+  // Extra data associated with a basic block
+  BlockDataType Data;
+};
+
+// The data saved for comparing functions.
+template <typename BlockDataType> class FuncDataTemplate {
+public:
+  FuncDataTemplate(const Function &F);
+
+  // Create data for a missing function that is present in the other IR
+  FuncDataTemplate(StringRef Name) { EntryBlockName = Name.str(); }
+
+  bool operator==(const FuncDataTemplate &That) const;
+
+  bool operator!=(const FuncDataTemplate &That) const {
+    return !(*this == That);
+  }
+
+  // Iterator over the saved basic block data
+  typename StringMap<BlockDataTemplate<BlockDataType>>::const_iterator
+  begin() const {
+    return Blocks.begin();
+  }
+  typename StringMap<BlockDataTemplate<BlockDataType>>::const_iterator
+  end() const {
+    return Blocks.end();
+  }
+
+  // Return the name of the entry block
+  std::string getEntryBlockName() const { return EntryBlockName; }
+
+  // Return the block data for the basic block with label \p S.
+  const BlockDataTemplate<BlockDataType> *
+  getBlockData(const StringRef S) const {
+    return (Blocks.count(S) == 1) ? &Blocks.find(S)->getValue() : nullptr;
+  }
+
+protected:
+  std::string EntryBlockName;
+  StringMap<BlockDataTemplate<BlockDataType>> Blocks;
+};
+
+// A map of names to the saved data.
+template <typename FuncDataType>
+class IRDataTemplate : public StringMap<FuncDataType> {
+public:
+  IRDataTemplate() = default;
+  bool operator==(const IRDataTemplate &That) const;
+  bool operator!=(const IRDataTemplate &That) const { return !(*this == That); }
+};
+
+enum IRChangeDiffType { InBefore, InAfter, IsCommon };
+
+// Abstract template base class for a class that compares two IRs.  The
+// class is created with the 2 IRs to compare and then compare is called.
+// The static function analyzeIR is used to build up the IR representation.
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+class IRComparer {
+protected:
+  IRComparer(const IRDataType &Before, const IRDataType &After, unsigned N);
+
+public:
+  virtual ~IRComparer();
+
+  // Compare the 2 IRs.  In the case of a module IR, delegate to
+  // handleFunctionCompare in the derived class for each function;
+  // otherwise delegate to handleCompare in the derived class.
+  void compare(Any IR, StringRef Prefix, StringRef PassID, StringRef Name);
+  // Analyze \p IR and save the data in \p Data.
+  static bool analyzeIR(Any IR, IRDataType &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(IRDataType &Data, const Function &F);
+
+  // Called to handle the compare of function when IR unit is a function.
+  virtual void handleSingleFunctionCompare(StringRef Name, StringRef Prefix,
+                                           StringRef PassID, unsigned,
+                                           unsigned *,
+                                           const FuncDataType &Before,
+                                           const FuncDataType &After) = 0;
+  // Called to handle the compare of function when IR unit is a module.
+  virtual void handleFunctionInModuleCompare(StringRef Name, StringRef Prefix,
+                                             StringRef PassID, unsigned,
+                                             unsigned *,
+                                             const FuncDataType &Before,
+                                             const FuncDataType &After) = 0;
+
+  const IRDataType &Before;
+  const IRDataType &After;
+  unsigned N;
+};
+
+// An empty class that satisfies the requirements for being a template
+// argument for the IRComparer template base class.
+class EmptyData {
+public:
+  static void initialize(EmptyData &, const BasicBlock &) {}
+};
+
+using EmptyDataBlockData = BlockDataTemplate<EmptyData>;
+using EmptyDataFuncData = FuncDataTemplate<EmptyData>;
+using EmptyDataIRData = IRDataTemplate<EmptyDataFuncData>;
+
+// A class that compares two IRs of type EmptyDataIRData and does a
+// linux 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).  The functions
+// handleCompare and handleFunctionCompare are called by the base
+// class with the appropriate parameters.
+class InLineComparer : public IRComparer<EmptyDataBlockData, EmptyDataFuncData,
+                                         EmptyDataIRData> {
+public:
+  InLineComparer(raw_ostream &OS, const EmptyDataIRData &Before,
+                 const EmptyDataIRData &After)
+      : IRComparer(Before, After, 0), Out(OS) {}
+  ~InLineComparer() override;
+
+  // Called to handle the compare of function when IR unit is a function.
+  void handleSingleFunctionCompare(StringRef Name, StringRef Prefix,
+                                   StringRef PassID, unsigned, unsigned *,
+                                   const EmptyDataFuncData &Before,
+                                   const EmptyDataFuncData &After) override;
+  // Called to handle the compare of function when IR unit is a module.
+  void handleFunctionInModuleCompare(StringRef Name, StringRef Prefix,
+                                     StringRef PassID, unsigned, unsigned *,
+                                     const EmptyDataFuncData &Before,
+                                     const EmptyDataFuncData &After) override;
+
+protected:
+  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<EmptyDataIRData> {
+public:
+  InLineChangePrinter() {}
+  ~InLineChangePrinter() override;
+  void registerCallbacks(PassInstrumentationCallbacks &PIC);
+
+protected:
+  // Create a representation of the IR.
+  virtual void generateIRRepresentation(Any IR, StringRef PassID,
+                                        EmptyDataIRData &Output) override;
+
+  // Called when an interesting IR has changed.
+  virtual void handleAfter(StringRef PassID, std::string &Name,
+                           const EmptyDataIRData &Before,
+                           const EmptyDataIRData &After, Any) override;
+  // Called to compare the before and after representations of the IR.
+  virtual bool same(const EmptyDataIRData &Before,
+                    const EmptyDataIRData &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 BlockDataTemplate<EmptyData>;
+extern template class FuncDataTemplate<EmptyData>;
+extern template class IRDataTemplate<EmptyDataFuncData>;
+extern template class ChangeReporter<EmptyDataIRData>;
+extern template class TextChangeReporter<EmptyDataIRData>;
+extern template class IRComparer<EmptyDataBlockData, EmptyDataFuncData,
+                                 EmptyDataIRData>;
+
 } // 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>
+    DiffForChangeReporters("diff-for-print-changed", cl::Hidden,
+                           cl::init("/usr/bin/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.";
+
+    llvm::raw_fd_ostream OutStream(FD[I], /*shouldClose=*/true);
+    if (FD[I] == -1)
+      return "Error opening file for writing.";
+    OutStream << SR[I];
+  }
+
+  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(DiffForChangeReporters, 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)) {
PIC.registerAfterPassCallback(
         handleIRAfterPass(IR, P);
       });
   PIC.registerAfterPassInvalidatedCallback(
       [this](StringRef P, const PreservedAnalyses &) {
         handleInvalidatedPass(P);
       });
 }
 
+template <typename BlockDataType>
+BlockDataTemplate<BlockDataType>::BlockDataTemplate(const llvm::BasicBlock &B)
+    : Label(B.getName().str()), Data() {
+  llvm::raw_string_ostream SS(Body);
+  B.Value::print(SS, true);
+  BlockDataType::initialize(Data, B);
+}
+
+template <typename IRUnitT>
+FuncDataTemplate<IRUnitT>::FuncDataTemplate(const llvm::Function &F) {
+  EntryBlockName = F.getEntryBlock().getName().str();
+  for (const auto &B : F)
+    Blocks.insert({B.getName(), B});
+}
+
+template <typename IRUnitT>
+bool FuncDataTemplate<IRUnitT>::operator==(const FuncDataTemplate &That) const {
+  for (auto &B : That) {
+    llvm::StringRef Label = B.getKey();
+    if (Blocks.count(Label) == 0)
+      return false;
+    const BlockDataTemplate<IRUnitT> &ThisData = Blocks.find(Label)->getValue();
+    const BlockDataTemplate<IRUnitT> &ThatData = B.getValue();
+    if (ThisData != ThatData)
+      return false;
+  }
+  return true;
+}
+
+template <typename IRUnitT>
+bool IRDataTemplate<IRUnitT>::operator==(const IRDataTemplate &That) const {
+  if (llvm::StringMap<IRUnitT>::size() != That.size())
+    return false;
+  for (auto &F : *this) {
+    llvm::StringRef Key = F.getKey();
+    if (That.count(Key) == 0 || F.getValue() != That.find(Key)->getValue())
+      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.
void IRChangedPrinter::handleAfter(StringRef PassID, std::string &Name,
 
   Out << After.substr(Banner.size());
 }
 
 bool IRChangedPrinter::same(const std::string &S1, const std::string &S2) {
   return S1 == S2;
 }
 
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+IRComparer<BlockDataType, FuncDataType, IRDataType>::IRComparer(
+    const IRDataType &Before, const IRDataType &After, unsigned N)
+    : Before(Before), After(After), N(N) {}
+
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+IRComparer<BlockDataType, FuncDataType, IRDataType>::~IRComparer() {}
+
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+void IRComparer<BlockDataType, FuncDataType, IRDataType>::compare(
+    llvm::Any IR, llvm::StringRef Prefix, llvm::StringRef PassID,
+    llvm::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.");
+    handleSingleFunctionCompare(Name, Prefix, PassID, N, nullptr,
+                                Before.begin()->getValue(),
+                                After.begin()->getValue());
+    return;
+  }
+  // Determine whether functions are common or not.
+  llvm::StringMap<IRChangeDiffType> Funcs;
+  for (auto &B : Before)
+    Funcs.insert({B.getKey(), InBefore});
+  for (auto &A : After)
+    if (Funcs.count(A.getKey()) == 1)
+      Funcs[A.getKey()] = IsCommon;
+    else
+      Funcs.insert({A.getKey(), InAfter});
+
+  // Set up an appropriate numbering system.
+  unsigned Minor = 0;
+
+  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 == InBefore) {
+      FuncDataType MissingFunc(
+          Before.find(Name)->getValue().getEntryBlockName());
+      handleFunctionInModuleCompare(Name, Prefix, PassID, N, &Minor,
+                                    Before.find(Name)->getValue(), MissingFunc);
+    } else if (CDDT == InAfter) {
+      FuncDataType MissingFunc(
+          After.find(Name)->getValue().getEntryBlockName());
+      handleFunctionInModuleCompare(Name, Prefix, PassID, N, &Minor,
+                                    MissingFunc, After.find(Name)->getValue());
+    } else {
+      assert(CDDT == IsCommon && "Unexpected Diff type.");
+      handleFunctionInModuleCompare(Name, Prefix, PassID, N, &Minor,
+                                    Before.find(Name)->getValue(),
+                                    After.find(Name)->getValue());
+    }
+    ++Minor;
+  }
+}
+
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+bool IRComparer<BlockDataType, FuncDataType, IRDataType>::analyzeIR(
+    llvm::Any IR, IRDataType &Data) {
+  if (const llvm::Module *M = getModule(IR)) {
+    // Create data for each existing/interesting function in the module.
+    bool OutputExists = false;
+    for (const llvm::Function &F : *M)
+      OutputExists = generateFunctionData(Data, F) | OutputExists;
+    return OutputExists;
+  }
+
+  const llvm::Function *F = nullptr;
+  if (llvm::any_isa<const llvm::Function *>(IR))
+    F = llvm::any_cast<const llvm::Function *>(IR);
+  else {
+    assert(llvm::any_isa<const llvm::Loop *>(IR) && "Unknown IR unit.");
+    const llvm::Loop *L = llvm::any_cast<const llvm::Loop *>(IR);
+    F = L->getHeader()->getParent();
+  }
+  assert(F && "Unknown IR unit.");
+  return generateFunctionData(Data, *F);
+}
+
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+const llvm::Module *
+IRComparer<BlockDataType, FuncDataType, IRDataType>::getModule(llvm::Any IR) {
+  if (llvm::any_isa<const llvm::Module *>(IR))
+    return llvm::any_cast<const llvm::Module *>(IR);
+  if (llvm::any_isa<const llvm::LazyCallGraph::SCC *>(IR)) {
+    const llvm::LazyCallGraph::SCC *C =
+        llvm::any_cast<const llvm::LazyCallGraph::SCC *>(IR);
+    for (const llvm::LazyCallGraph::Node &N : *C)
+      return N.getFunction().getParent();
+  }
+  return nullptr;
+}
+
+template <typename BlockDataType, typename FuncDataType, typename IRDataType>
+bool IRComparer<BlockDataType, FuncDataType, IRDataType>::generateFunctionData(
+    IRDataType &Data, const llvm::Function &F) {
+  if (!F.isDeclaration() && llvm::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,
+                                                   EmptyDataIRData &D) {
+  InLineComparer::analyzeIR(IR, D);
+}
+
+void InLineChangePrinter::handleAfter(StringRef PassID, std::string &Name,
+                                      const EmptyDataIRData &Before,
+                                      const EmptyDataIRData &After, Any IR) {
+  if (Name == "")
+    Name = " (module)";
+  SmallString<20> Banner =
+      formatv("*** IR Dump After {0} ***{1}\n", PassID, Name);
+  Out << Banner;
+  InLineComparer(Out, Before, After).compare(IR, "", PassID, Name);
+  Out << "\n";
+}
+
+bool InLineChangePrinter::same(const EmptyDataIRData &D1,
+                               const EmptyDataIRData &D2) {
+  return D1 == D2;
+}
+
+InLineComparer::~InLineComparer() {}
+
+void InLineComparer::handleSingleFunctionCompare(
+    llvm::StringRef, llvm::StringRef Prefix, llvm::StringRef PassID, unsigned,
+    unsigned *, const EmptyDataFuncData &Before,
+    const EmptyDataFuncData &After) {
+  for (const auto &B : Before) {
+    const EmptyDataBlockData &BeforeBD = B.getValue();
+    const EmptyDataBlockData *BD = After.getBlockData(BeforeBD.getLabel());
+    if (BD)
+      // This block is in both
+      Out << doSystemDiff(BeforeBD.getBody(), BD->getBody(), "-%l\n", "+%l\n",
+                          " %l\n");
+    else
+      // This has been removed
+      Out << doSystemDiff(BeforeBD.getBody(), "\n", "-%l\n", "+%l\n", " %l\n");
+  }
+  for (const auto &B : After) {
+    const EmptyDataBlockData &AfterBD = B.getValue();
+    const EmptyDataBlockData *BD = Before.getBlockData(AfterBD.getLabel());
+    if (!BD)
+      // This block is new
+      Out << doSystemDiff("\n", AfterBD.getBody(), "-%l\n", "+%l\n", " %l\n");
+  }
+}
+
+void InLineComparer::handleFunctionInModuleCompare(
+    llvm::StringRef Name, llvm::StringRef Prefix, llvm::StringRef PassID,
+    unsigned, unsigned *, const EmptyDataFuncData &Before,
+    const EmptyDataFuncData &After) {
+  Out << "\n*** IR for function " << Name << " ***\n";
+  InLineComparer::handleSingleFunctionCompare(Name, Prefix, PassID, 0, nullptr,
+                                              Before, After);
+}
+
+void InLineChangePrinter::registerCallbacks(PassInstrumentationCallbacks &PIC) {
+  if (PrintChangedDiff)
+    TextChangeReporter<EmptyDataIRData>::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 BlockDataTemplate<EmptyData>;
+template class FuncDataTemplate<EmptyData>;
+template class IRDataTemplate<EmptyDataFuncData>;
+template class ChangeReporter<EmptyDataIRData>;
+template class TextChangeReporter<EmptyDataIRData>;
+template class IRComparer<EmptyDataBlockData, EmptyDataFuncData,
+                          EmptyDataIRData>;
+
 } // 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 ***