[PATCH] D114564: Fix the use of -fno-approx-func along with -Ofast or -ffast-math

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor accepted this revision.
andrew.w.kaylor added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2760
     case options::OPT_fno_honor_nans:       HonorNaNs = false;        break;
     case options::OPT_fapprox_func:         ApproxFunc = true;        break;
     case options::OPT_fno_approx_func:      ApproxFunc = false;       break;
----------------
joerg wrote:
> masoud.ataei wrote:
> > masoud.ataei wrote:
> > > andrew.w.kaylor wrote:
> > > > Should this also imply "MathErrno = false"?
> > > I don't think setting ApproxFunc to true should imply "MathErrno = false". 
> > > 
> > > Let say someone have a math library that compute approximate result for none special input/output but returns NaN, INF and errno correctly otherwise. That is actually can be fairly common, because performance in the none special cases are much more important that the special ones. So returning errno in the special outputs theoretically should not effect the performance on the main path. Therefore, I think compiler should not assume anything about MathErrno value based on ApproxFunc value.
> > I am not sure what I was suggesting in my last comment is correct or not. Can one of the more experienced reviewers confirm?
> > The question is: Should "ApproxFunc=ture" implies "MathErrno=false"?
> They seem pretty orthogonal to me.
I see the point of your explanation. I'm satisfied that they should remain separate.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114564/new/

https://reviews.llvm.org/D114564