aaron.ballman added a reviewer: efriedma.
aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/FPOptions.def:29
 OPTION(Float16ExcessPrecision, LangOptions::ExcessPrecisionKind, 2, 
FPEvalMethod)
 OPTION(BFloat16ExcessPrecision, LangOptions::ExcessPrecisionKind, 2, 
FPEvalMethod)
+OPTION(MathErrno, bool, 1, BFloat16ExcessPrecision)
----------------
Shouldn't this one be `Float16ExcessPrecision`? (Are we missing test coverage 
that would have caught that?)


================
Comment at: clang/include/clang/Basic/LangOptions.h:857
     setAllowApproxFuncOverride(!Value);
+    setMathErrnoOverride(Value);
     if (Value)
----------------
Everything else does `!Value`; is it intentional that you're using `Value` 
instead?


================
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.
----------------
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?


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

https://reviews.llvm.org/D151834

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to