[PATCH] D137578: Fix 'unsafe-math-optimizations' flag dependencies

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3957
 
-  if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
-    StringRef Val = A->getValue();
-    if (Val == "fast")
+  if (auto *A = Args.getLastArg(OPT_ffp_contract, OPT_cl_fast_relaxed_math,
+                                OPT_cl_unsafe_math_optimizations)) {
----------------
michele.scandale wrote:
> If I look only at the CC1 behavior then I think this is probably the desired behavior.
> 
> However from the compiler driver perspective it seems that `-ffp-contract` and `-cl-*` math options don't really play well together -- in `RenderFloatingPointOptions` the OpenCL related options are not considered, and similarly in `RenderOpenCLOptions` the other floating point options are not considered.
> 
> It's not clear to me if it is expected to mix OpenCL specific options with other floating point related options -- given todays code it seems that is not expected.
> For the time being it is probably better to simply keep the `ffp_contract` handling as is today, and modify the code around line 3803 as follow
> ```
>   if (Opts.FastRelaxedMath || Opts.CLUnsafeMath)
>     Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
> ```
> to handle `-cl-unsafe-math-optimizations` similarly to `-cl-fast-relaxed-math` w.r.t. contraction behavior.
>>
It's not clear to me if it is expected to mix OpenCL specific options with other floating point related options -- given todays code it seems that is not expected.
>>
Yes, it looks like the FP options and CL options are orthogonal. Even so not sure how OpenCL people will react to the LIT test behavioral change (relaxed-fpmath.cl). Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137578