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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

I'm actually very excited for this, it makes tracking down a bad transform easier and would have helped in earlier investigations!



================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:59
+// modules if -print-module-scope is specified) for a particular function
+// or indicating that the IR has been filtered out.  The exta options
+// can be combined, allowing only changed IRs for certain passes on certain
----------------



================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:229
+bool isInterestingFunction(const Function &F) {
+  return !F.isDeclaration() && llvm::isFunctionInPrintList(F.getName());
+}
----------------
I don't think this is necessary since the ModuleToFunctionPassAdaptor already filters declarations out.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:69
+// 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"),
----------------
jamieschmeiser wrote:
> ychen wrote:
> > Maybe use a pass's `PassRegistery.def` name instead of its textual class name? It would be consistent.
> While it may be more consistent, I think it would also be more inconvenient for the user and would require the user to either know the PassRegistry.def id or look it up.  However, with the textual class name, it can be determined by looking at the output itself.  So, if the output is being examined and it is determined that a pass can be filtered out, the name of the pass is there in the output.  Also, to use the PassRegistry.def id would require a map to get back to the textual class name as that is what the callbacks provide and would require updating the mapping function each time a pass name changed or a new pass was added or deleted.
I'm also in favor of using the short name like "instsimplify" that matches --print-before/after, and -passes=, it seems more consistent. If you're looking at IR, you probably are familiar with how to run passes under `opt`, which requires knowing the short name.

However, that doesn't work under the NPM yet for --print-before/after, see https://bugs.llvm.org/show_bug.cgi?id=47370.

So for now we can proceed with this, but I'll probably try and fix this up alongside --print-before/after later.


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

https://reviews.llvm.org/D86360