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() == ---------------- 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`. ================ Comment at: clang/test/CodeGen/func-attr.c:5 // RUN: %clang -c -target x86_64 -funsafe-math-optimizations \ -// RUN: -emit-llvm -S -o - %s | FileCheck %s +// RUN: -fno-math-errno -ffp-contract=fast -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE + ---------------- See comment about `MathErrno` in `CGCall.cpp` 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