michele.scandale added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869 + if ((LangOpts.FastMath || + !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip && + !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero && + LangOpts.ApproxFunc) && + LangOpts.getDefaultFPContractMode() != LangOptions::FPModeKind::FPM_Off) ---------------- zahiraam wrote: > michele.scandale wrote: > > If I look at the clang driver code, `-funsafe-math-optimizations` is > > specified on the CC1 command line if > > ``` > > !MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && > > ApproxFunc && !TrappingMath > > ``` > > is true. > > This means that it shouldn't matter the value of the floating point > > contraction, or whether infs/nans are honored or not. > > > > Was there another issue that this specific part of the change addresses? > This is to make sure that if we are using -ffast-math or > -funsafe-math-optimization along with -ffp-contract=off, fma instructions are > not generated. In this case the function shouldn't be marked with the > unsafe-fp-math attribute. > This is to make sure that if we are using -ffast-math or > -funsafe-math-optimization along with -ffp-contract=off, fma instructions are > not generated. In this case the function shouldn't be marked with the > unsafe-fp-math attribute. If I understand correctly this is because the function attribute `unsafe-fp-math` from backends perspective allow any kind of contraction? If so, shouldn't then this be valid only when the fp-contraction mode is `fast`, given that `on` means that only in-statement contraction is allowed, and clang handles that by generating `llvm.fmuladd` to lower expressions like `a * b + c` in given statement? What about the `!LangOpts.FiniteMathOnly` part? I'd would think this shouldn't be checked at all as there are separate function attributes for infs and nans. ================ Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869 + if ((LangOpts.FastMath || + !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip && + !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero && + LangOpts.ApproxFunc) && + LangOpts.getDefaultFPContractMode() != LangOptions::FPModeKind::FPM_Off) ---------------- michele.scandale wrote: > zahiraam wrote: > > michele.scandale wrote: > > > If I look at the clang driver code, `-funsafe-math-optimizations` is > > > specified on the CC1 command line if > > > ``` > > > !MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && > > > ApproxFunc && !TrappingMath > > > ``` > > > is true. > > > This means that it shouldn't matter the value of the floating point > > > contraction, or whether infs/nans are honored or not. > > > > > > Was there another issue that this specific part of the change addresses? > > This is to make sure that if we are using -ffast-math or > > -funsafe-math-optimization along with -ffp-contract=off, fma instructions > > are not generated. In this case the function shouldn't be marked with the > > unsafe-fp-math attribute. > > This is to make sure that if we are using -ffast-math or > > -funsafe-math-optimization along with -ffp-contract=off, fma instructions > > are not generated. In this case the function shouldn't be marked with the > > unsafe-fp-math attribute. > > If I understand correctly this is because the function attribute > `unsafe-fp-math` from backends perspective allow any kind of contraction? > If so, shouldn't then this be valid only when the fp-contraction mode is > `fast`, given that `on` means that only in-statement contraction is allowed, > and clang handles that by generating `llvm.fmuladd` to lower expressions like > `a * b + c` in given statement? > > What about the `!LangOpts.FiniteMathOnly` part? I'd would think this > shouldn't be checked at all as there are separate function attributes for > infs and nans. > This is to make sure that if we are using -ffast-math or > -funsafe-math-optimization along with -ffp-contract=off, fma instructions are > not generated. In this case the function shouldn't be marked with the > unsafe-fp-math attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135097/new/ https://reviews.llvm.org/D135097 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
