[PATCH] D151834: Include math-errno with fast-math

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2321-2328
+  if ((FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone) ||
       ((ConstWithoutErrnoAndExceptions || ConstWithoutExceptions) &&
-       (!ConstWithoutErrnoAndExceptions || (!getLangOpts().MathErrno)))) {
+       (!ConstWithoutErrnoAndExceptions ||
+        (!getLangOpts().MathErrno && !ErrnoOverriden && !OptNone) ||
+        // If math-errno was enabled on command line but overriden to false
+        // via '#pragma float_control(precise, off))', and optimizations are
+        // enabled, generate intrinsics.
----------------
aaron.ballman wrote:
> zahiraam wrote:
> > aaron.ballman wrote:
> > > I think this has gotten sufficiently complex that it might be worth splitting the logic out a bit more:
> > > ```
> > > bool Optimize = FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone;
> > > if (!Optimize) Optimize = FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone;
> > > ... and so on ...
> > > ```
> > > WDYT?
> > I understand I need to split the logic, but I don't get your proposal here. Do you mean?
> > 
> > 
> > ```
> > bool Optimize = FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone;
> > if (!Optimize) Optimize = !getLangOpts().MathErrno && !ErrnoOverriden && !OptNone;
> > ... and so on ...
> > ```
> > 
> Oops, yeah, that a was copy pasta mistake on my part. You've got the right idea -- mostly just split up the logic and add comments explaining why the predicates exist.
Changed the name of the variable you proposed. Is that OK?


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

https://reviews.llvm.org/D151834