[PATCH] D86360: Add new hidden option -print-changed which only reports changes to IR

[Fangrui Song via Phabricator via llvm-commits]

MaskRay requested changes to this revision.
MaskRay added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:131
+  // Is this the first IR seen?
+  bool InitialIR;
+};
----------------
Prefer member initializer

InitialIR = true


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:47
 
+// A hidden option that prints out the IR after passes, similar to
+// -print-after-all except that it only prints the IR after passes that
----------------
No need to specifically to call out "hidden". Other options don't document themselves "hidden".


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:52
+// also reported.  Other hidden options affect the output from this
+// option.  -filter-passes will limit the output to the named passes
+// that actually change the IR and other passes are reported as filtered out.
----------------
The relations with -filter-passes and print-module-scope probably should be moved to a separate graph, because several other `-print-*` (e..g `-print-after-all`) share the documentation.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:274
+  // Save the IR representation on the stack.
+  auto &Data = BeforeStack.back();
+  generateIRRepresentation(IR, PassID, Data);
----------------



================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:291
+  }
+  if (Name == "")
+    Name = " (module)";
----------------



================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:305
+
+    // was there a change in IR?
+    if (same(Before, After))
----------------
Was


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:306
+    // was there a change in IR?
+    if (same(Before, After))
+      omitAfter(PassID, Name);
----------------
same has a trivial definition. Consider deleting the member function


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:356
+  unwrapAndPrint(Out, IR, Banner, true,
+                 /*Brief*/ false, /*ShouldPreserveUseListOrder*/ true);
+}
----------------
Prefer the argument comment form detectable by https://clang.llvm.org/extra/clang-tidy/checks/bugprone-argument-comment.html


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:407
+                           const std::string &After) {
+  return Before.compare(After) == 0;
+}
----------------
Before == After


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

https://reviews.llvm.org/D86360