michele.scandale added a comment. 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? 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
