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