[PATCH] D138456: Add new option choices for print-changed that do not output llc changes.

[Jamie Schmeiser via Phabricator via llvm-commits]

jamieschmeiser added inline comments.


================
Comment at: llvm/include/llvm/IR/PrintPasses.h:37
+  OptDotCfgVerbose,
+  OptDotCfgQuiet
 };
----------------
aeubanks wrote:
> MaskRay wrote:
> > The number of modes is massive. Shall we use another option to indicate the intent (only show non-codegen passes)?
> agreed, this is growing exponentially because it really should be multiple flags
> 
> something like
> `print-changed`
> `print-changed-diff[=color]`
> `print-ir-pass-changed` (default true)
> `print-mir-pass-changed` (default true)
> `print-dotcfg`
> `print-changed-verbose` (affects all change printers)
> ...
> 
> this will allow users to specify more than one change printer at once which I think is fine (could manually error out if not desired, but I don't think that's necessary)
My first thought was to change the llc versions to print-llc-changed but I didn't want to propose changing a functionality that others may want (ie, having both come out with one option).  I think having the two piggy-back on the same option is too much.  The filtering options can be overloaded.  What about the following?
print-changed[=diff|=cdiff] (both)
print-ir-changed[=diff|=cdiff]
print-mir-changed[=diff|=cdiff]
print-dotcfg
print-changed-verbose (defaults to false, affects all change printers)

The print-changed-verbose default could also be controlled by an environment variable 
```
__LLVM_PRINT_CHANGED_VERBOSE_DEFAULT__==TRUE
``` 
(is there precedence?, better name?) 


================
Comment at: llvm/lib/CodeGen/MachineFunctionPass.cpp:131
       switch (PrintChanged) {
-      case ChangePrinter::None:
+      default:
         llvm_unreachable("");
----------------
MaskRay wrote:
> I think we should keep enumerating all values to ensure a new mode does not silently get unhandled here.
Fine with me.  I'll work that up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138456