[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

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

dexonsmith added a comment.

In D82756#2350233 <https://reviews.llvm.org/D82756#2350233>, @jansvoboda11 wrote:

> Correct me if I'm wrong, but when generating the command line, all "implied" flags would be hidden, even if they were explicit in the original comand line:
>
> - original command line:  `clang -cc1 -cl-unsafe-math-optimizations -cl-mad-enable -menable-unsafe-fp-math -mreassociate -fno-signed-zeros -freciprocal-math -fapprox-func [...]`
> - generated command line: `clang -cc1 -cl-unsafe-math-optimizations [...]`
>
> This might be a bit surprising, but I don't think this would cause issues for explicit modules. What are your thoughts?

I think this is fine. It's similar to a case where the caller might explicitly specify the default value, and it'll get canonicalized out.

> Formalizing the "implies" relationships would make it possible to remove the ordering-sensitivity and possibly generate implied flags even when explicitly passed to `cc1`. It would complicate the TableGen backend, which I'd prefer to keep as simple as possible.

I was't thinking of dropping the ordering-sensitivity. Instead, you could just error if the referenced option hadn't declared already. One idea would be to change the tablegen to something like:

  MarshallingInfoFlag<
      "CodeGenOpts.LessPreciseFPMAD",
      DefaultAnyOf<[cl_unsafe_math_optimizations, cl_fast_relaxed_math]>>;

in the definition of `cl_mad_enable`, then:

- error if they aren't defined first; and
- construct a default value out of the key-paths.

I think this would less error-prone for maintenance, since it designs away some really subtle bugs.


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

https://reviews.llvm.org/D82756