[PATCH] D83892: [clang][cli] Port CodeGen option flags to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D83892#2458049 <https://reviews.llvm.org/D83892#2458049>, @probinson wrote:

> One thing this patch does, is make decisions about default behavior static.  Meaning, the option behavior cannot depend on other options; specifically, it can't be based on the triple, which allows target-specific customization.  PS4 certainly has cases where our defaults are different from the usual ones, and I'd kind of think that was true for other targets as well.
>
> Sorry I didn't notice this patch before, our CI has just tried to merge it.  We've patched it up in our main branch but I'm not sure what the upstream intent is here.

`BoolOption` doesn't support dynamic defaults, but `BoolOptionBase` does. Here's an example of how to use it:

  defm legacy_pass_manager : BoolOptionBase<"legacy-pass-manager",
    "CodeGenOpts.LegacyPassManager", Default<"!static_cast<unsigned>(LLVM_ENABLE_NEW_PASS_MANAGER)">,
    FlagDef<PosFlag, true, [], "Use the legacy pass manager in LLVM">,
    FlagDef<NegFlag, false, [], "Use the new pass manager in LLVM">,
    FlagDefSuffix<[CC1Option], "">, "f">, Group<f_clang_Group>;

For depending on `-triple`, you should be able to do something like:

  defm this_option : BoolOptionBase<...,
      Default<"getDefaultForThisOption(TargetOptions->Triple)",
      ...>

if I understand your use case correctly (it's important to define `this_option` after the definition of `triple`).

Maybe it's worth defining another set of derived multiclasses, something like:

  defm legacy_pass_manager : DynamicBoolFOption<"legacy-pass-manager",
    "CodeGenOpts.LegacyPassManager", Default<"!static_cast<unsigned>(LLVM_ENABLE_NEW_PASS_MANAGER)">,
    TrueFlag<PosFlag, [], "Use the legacy">,
    FalseFlag<NegFlag, [], "Use the new">,
    BothFlags<[], " pass manager in LLVM">>;

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83892