michele.scandale added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:1865 FuncAttrs.addAttribute("approx-func-fp-math", "true"); - if ((LangOpts.FastMath || - (!LangOpts.FastMath && LangOpts.AllowFPReassoc && - LangOpts.AllowRecip && !LangOpts.FiniteMathOnly && - LangOpts.NoSignedZero && LangOpts.ApproxFunc)) && - LangOpts.getDefaultFPContractMode() != LangOptions::FPModeKind::FPM_Off) + if (LangOpts.UnsafeFPMath && + (LangOpts.getDefaultFPContractMode() == ---------------- michele.scandale wrote: > I've found quite confusing the `(FastMath || (!FastMath && ... ))`. > > Using directly `UnsafeFPMath` seems more compact, however it also causes to > taking into account the value for `MathErrno` -- which it might be not > relevant. > > If `MathErrno` shouldn't affect this, then I would rewrite this as: > ``` > if (LangOpts.AllowFPReassoc && LangOpts.AllowRecip && > LangOpts.NoSignedZero && LangOpts.ApproxFunc && > (LangOpts.getDefaultFPContractMode() == > LangOptions::FPModeKind::FPM_Fast || > LangOpts.getDefaultFPContractMode() == > LangOptions::FPModeKind::FPM_FastHonorPragmas)) > FuncAttrs.addAttribute("unsafe-fp-math", "true"); > ``` > so that only the relevant options are considered and there is no need to > think about what is implied by `FastMath`. I do wonder if in https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3089-L3091 is actually correct to have `!MathErrno`. In the GCC documentation of `-funsafe-math-optimizations` I don't see any connection to the `-fmath-errno` or `-fno-math-errno` options. 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