[PATCH] D87000: Add a new hidden option -print-changes that reports in a patch-like notation when a pass actually changes the IR.

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added inline comments.


================
Comment at: llvm/include/llvm/IR/ChangeReporters.h:258
+public:
+  // Comare the 2 IRs.  In the case of a module IR, delegate to
+  // handleFunctionCompare in the derived class for each function;
----------------
Typo: compare


================
Comment at: llvm/lib/IR/ChangeReporters.cpp:150
+    : Label(B.getName().str()), Data() {
+  std::string Raw;
+  std::string Raw1;
----------------
You are not using this variable.


================
Comment at: llvm/lib/IR/ChangeReporters.cpp:187
+    if (F.getValue() != that.find(Key)->getValue())
+      return false;
+  }
----------------
can combine above two "if"s with logical or and return false.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:147
+bool ExecuteCommand(const StringRef Cmd, std::string *CmdOutput) {
+  FILE *Pipe = popen(Cmd.str().c_str(), "r");
+  if (!Pipe)
----------------
Why not use standard C++ file handling? This is old C's way of manipulating files.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:171
+  static int FD[2]{-1, -1};
+  for (int I = 0; I < 2; ++I) {
+    if (FD[I] == -1) {
----------------
I can be unsigned.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87000/new/

https://reviews.llvm.org/D87000