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

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked an inline comment as done.
dang added inline comments.


================
Comment at: clang/test/CodeGen/fp-function-attrs.cpp:2
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffinite-math-only -menable-unsafe-fp-math \
+// RUN:   -menable-no-infs -menable-no-nans -fno-signed-zeros -freciprocal-math \
+// RUN:   -fapprox-func -mreassociate -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
----------------
Anastasia wrote:
> dang wrote:
> > Anastasia wrote:
> > > Not clear why do you need to pass these extra flags now?
> > Previously passing -ffast-math to CC1 implied all these other flags. I am trying to make CC1 option parsing as simple as possible, so that we can then make it easy to generate a command line from a CompilerInvocation instance. You can refer to [[ http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html | http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html ]] for more details on why we want to be able to do this
> Just to understand, there used to be implied flags and it made the manual command line use of clang more compact and easy... Is the idea now to change those compound flags such that individul flags always need to be passed?
> 
> Although I thought you are still adding the implicit flags:
> 
>           {options::OPT_cl_fast_relaxed_math,
>            [&](const Arg *Arg) {
>              RenderArg(Arg);
> 
>              CmdArgs.push_back(GetArgString(options::OPT_cl_mad_enable));
>              CmdArgs.push_back(GetArgString(options::OPT_ffast_math));
>              CmdArgs.push_back(GetArgString(options::OPT_ffinite_math_only));
>              CmdArgs.push_back(
>                  GetArgString(options::OPT_menable_unsafe_fp_math));
>              CmdArgs.push_back(GetArgString(options::OPT_mreassociate));
>              CmdArgs.push_back(GetArgString(options::OPT_menable_no_nans));
>              CmdArgs.push_back(
>                  GetArgString(options::OPT_menable_no_infinities));
>              CmdArgs.push_back(GetArgString(options::OPT_fno_signed_zeros));
>              CmdArgs.push_back(GetArgString(options::OPT_freciprocal_math));
>              CmdArgs.push_back(GetArgString(options::OPT_fapprox_func));
>            }}
> 
> Do I just misunderstand something?
The command line of the driver doesn't change. This patch only affects what CC1 understands, now CC1 doesn't know anymore that `-cl-fast-relaxed-math` implies all these other options so the driver is responsible for specifying them when it constructs the CC1 command line.

To summarize, the clang driver command line isn't affected by this patch and it shouldn't be so let me know if something is wrong there. However, manually constructed `clang -cc1` invocations need to specify the all the implied flags manually now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82756