We cannot be sure whether a flag is CC1Option inside the definition of BoolOption. Take the example below:
let Flags = [CC1Option] in {
defm xxx : BoolOption<...>;
}where TableGen applies Flags = [CC1Option] to the xxx and no_xxx records after they have been is fully declared by BoolOption.
For the refactored -f[no-]debug-pass-manager flags (see the diff), this means BoolOption never adds any marshalling info, as it doesn't see either of the flags as CC1Option.
For that reason, we should defensively append the marshalling information to both flags inside BoolOption. Now the check for CC1Option needs to happen later, in the parsing macro, when all TableGen logic has been resolved.
However, for some flags defined through BoolOption, we can run into issues:
// Options.td def fenable_xxx : /* ... */; // Both flags are CC1Option, the first is implied. defm xxx : BoolOption<"xxx, "Opts.Xxx", DefaultsToFalse, ChangedBy<PosFlag, [CC1Option], "", [fenable_xxx]>, ResetBy<NegFlag, [CC1Option]>>;
When parsing clang -cc1 -fenable-xxx:
- we run parsing for PosFlag:
- set Opts.Xxx to default false,
- discover PosFlag is implied by -fenable-xxx: set Opts.Xxx to true,
- don't see -fxxx on command line: do nothing,
- we run parsing for NegFlag:
- set Opts.Xxx to default false,
- discover NegFlag cannot be implied: do nothing,
- don't see -fno-xxx on command line: do nothing.
Now we ended up with Opts.Xxx set to false instead of true. For this reason, we need to ensure to append the same ImpliedByAnyOf instance to both flags.
This means both parsing runs now behave identically (they set the same default value, run the same "implied by" check, and call makeBooleanOptionNormalizer that already has information on both flags, so it returns the same value in both calls).
The solution works well, but what might be confusing is this: you have defined a flag A that is not CC1Option, but can be implied by another flag B that is CC1Option:
- if A is defined manually, it will never get implied, as the code never runs
def no_signed_zeros : Flag<["-"], "fno-signed-zeros">, Group<f_Group>, Flags<[]>, MarshallingInfoFlag<"LangOpts->NoSignedZero">, ImpliedByAnyOf<[menable_unsafe_fp_math]>;
- if A is defined by BoolOption, it could get implied, as the code is run by its CC1Option counterpart:
defm signed_zeros : BoolOption<"signed-zeros",
"LangOpts->NoSignedZero", DefaultsToFalse,
ChangedBy<NegFlag, [], "Allow optimizations that ignore the sign of floating point zeros",
[cl_no_signed_zeros, menable_unsafe_fp_math]>,
ResetBy<PosFlag, [CC1Option]>, "f">, Group<f_Group>;This is a surprising inconsistency.
One solution might be to somehow propagate the final Flags of the implied flag in ImpliedByAnyOf and check whether it has CC1Option in the parsing macro. However, I think it doesn't make sense to spend time thinking about a corner case that won't come up in real code.
An observation: it is unfortunate that the marshalling information is a part of the flag definition. If we represented it in a separate structure, we could avoid the "double parsing" problem by having a single source of truth. This would require a lot of additional work though.
Note: the original patch missed the CC1Option check in the parsing macro, making my reasoning here incomplete. Moreover, it contained a change to denormalization that wasn't necessarily related to these changes, so I've extracted that to a follow-up patch: D93094.