[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

[Michele Scandale via Phabricator via cfe-commits]

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