[PATCH] D138456: Add new option choices for print-changed that do not output llc changes.
Jamie Schmeiser via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 2 08:14:31 PST 2022
jamieschmeiser marked an inline comment as not done.
jamieschmeiser added inline comments.
================
Comment at: llvm/include/llvm/IR/PrintPasses.h:37
+ OptDotCfgVerbose,
+ OptDotCfgQuiet
};
----------------
MaskRay wrote:
> jamieschmeiser wrote:
> > 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?)
> An environment variable for a debugging feature is too much. Users should know that the functionality is unstable like any other API, and perhaps more unstable since this is a debugging feature.
@MaskRay @aeubanks I was just suggesting the environment variable as a way of customizing the option to avoid typing and am fine with dropping that idea. I am more interested in agreement about the proposed options. If there is concensus, I will make the appropriate changes but it makes sense to me to get agreement before spending the time making the code changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138456/new/
https://reviews.llvm.org/D138456
More information about the llvm-commits
mailing list