zahiraam added a comment. In D136786#3907235 <https://reviews.llvm.org/D136786#3907235>, @michele.scandale wrote:
> In D136786#3903646 <https://reviews.llvm.org/D136786#3903646>, @zahiraam > wrote: > >> The changes in this patch look good to me. >> @michele.scandale please make sure not to drop the driver changes that we >> agreed upon in this patch. Thanks. > > I started looking at the change needed to have `unsafe-math => > fp-contract=fast`. > > If I look how `-ffast-math` behave today, I see that in the driver code > `-ffast-math` changes the state for `FPContract` > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3032), > but then the condition for which `-ffast-math` is forwarded to the CC1 > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3149) > doesn't consider `FPContract`. This seems related to the fact that GCC does > emit `__FAST_MATH__` even if the contraction mode is not fast. > From a quick look the `FastMath` language options is used mainly to guard > some macro definition and a codegen path for complex floating point values, > so this seems ok in practice. > > It seems intended that the semantic of `-ffast-math` for the CC1 is different > than the semantic of `-ffast-math` for the compiler driver. Based on this, > I'd expect a similar solution for `-funsafe-math-optimizations`, i.e. > `-funsafe-math-optimizations => -ffp-contract=fast` only at the compiler > driver level. Does this sound good? > > If so, then the driver change for the `-funsafe-math-optimizations -> > -ffp-contract=fast` could be done separately without affecting the code here. > >> I talked to @andrew.w.kaylor offline: I was thinking that it might be >> necessary to make the two driver changes we talked about, before merging >> this patch. But if the tests pass then I think it's okay to implement the >> driver changes in an upcoming patch. > > In this patch the only suboptimal test change is the change to > `clang/test/CodeGenOpenCL/relaxed-fpmath.cl`, where despite the presence of > `-cl-unsafe-math-optimizations` the `"unsafe-fp-math"="true"` function > attributes is not generated due to the contraction mode not being fast. > My understanding is that `-cl-fast-relaxed-math` should be equivalent to > `-ffast-math`, and `-cl-unsafe-math-optimizations` should be equivalent to > `-funsafe-math-optimizations` from the user perspective. > > From what I see the `-cl-*` options are simply forwarded as-is to the CC1, > and this seems to be the desired behavior for the compiler driver w.r.t. > OpenCL specific options. From what I see > https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L3803 > implements `-cl-fast-relaxed-math => -ffast-math`, but this solution doesn't > play well if on the same command line you also have `-ffp-contract=VAL` as > the relative order of the options is not taken into account. I'd expect a > similar change for the `-cl-unsafe-math-optimizations` case. > > I can either put a `TODO` comment in the > `clang/test/CodeGenOpenCL/relaxed-fpmath.cl` test, and make the changes with > the driver changes for `-funsafe-math-optimizations`, otherwise the change > for the `-cl-unsafe-math-optimizations` options needs to be done in this > patch. > > Any preference? That's exactly what I was worried about. I think it would be best to create another patch to make the 'funsafe-math-optimization' => FpContract=fast and make sure we have coherence with 'ffast-math' and the cl options, then check in this patch once that's done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136786/new/ https://reviews.llvm.org/D136786 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
