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

[Daniel Grumberg via Phabricator via llvm-commits]

dang added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:1176
+defm reciprocal_math : OptInFFlag< "reciprocal-math", "Allow division operations to be reassociated", "", "", [], "LangOpts->AllowRecip">;
+def fapprox_func : Flag<["-"], "fapprox-func">, Group<f_Group>, Flags<[CC1Option, NoDriverOption]>,
+  MarshallingInfoFlag<"LangOpts->ApproxFunc", "false">;
----------------
Anastasia wrote:
> could this also be OptInFFlag?
The aim was to keep the driver semantics the same as before and this was not something you could control with the driver, so I left it as just a CC1 flag. However if it makes sense to be able to control this from the driver then we can definitely make this `OptInFFLag`.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2805
     CmdArgs.push_back("-menable-unsafe-fp-math");
+    ApproxFunc = true;
+  }
----------------
Anastasia wrote:
> Is this a bug fix ?
No, in current trunk approximating floating point functions was something that was implied by other optimization flags, i.e. disabling math errno, enabling associative/reciprocal math, disabling signed zeros and disabling trapping math and -ffast-math which does all the previously mentioned things. This patch moves this logic in the driver by introducing a new CC1 flag for this so that parsing CC1 options can be more easily automated. This just reflects the logic that was previously inside cc1.


================
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:
> 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82756