jcranmer-intel added a comment. In D134369#3821949 <https://reviews.llvm.org/D134369#3821949>, @efriedma wrote:
> I think __builtin_fmax can raise a floating-point exception; in that case, it > wouldn't be constant, I think? Not sure how consistent we are about handling > that sort of thing in constant evaluation at the moment. https://eel.is/c++draft/library.c#3 says that a floating-point exception other than `FE_INEXACT` causes it to not be a constant expression, if I have constant expression wording right. `FE_INVALID` can be raised if the inputs are signalling NaNs, but even if we're claiming implementation of Annex F, it's okay to treat sNaN as qNaN unless we're claiming `FE_SNANS_ALWAYS_SIGNAL` (this is new in C2x, I think). We shouldn't be claiming that except maybe if we're using `-fp-model=strict`. I don't think `#pragma STDC FENV_ACCESS ON` bears any relevance here, but now I'm wondering what needs to happen with that for regular floating-point operations that may trigger exceptions. ================ Comment at: clang/lib/AST/ExprConstant.cpp:14033-14034 + !EvaluateFloat(E->getArg(1), RHS, Info)) + return false; + if (Result.isNaN() || RHS > Result) + Result = RHS; ---------------- If I'm reading APFloat's source correctly, this doesn't suffice to consistently make __builtin_fmax(±0.0, ±0.0) return +0.0 if one of the zeroes is positive. (We're not required to return +0.0 in this case, but most libm implementations seem to endeavor to return +0.0 in this situation, even if the LLVM intrinsic we lower to doesn't). ================ Comment at: clang/test/Sema/constant-builtins-fmax.cpp:35-39 +#define FMAX_TEST_BOTH_ZERO(T, FUNC) \ + static_assert(0.0 == FUNC(0.0, 0.0)); \ + static_assert(0.0 == FUNC(-0.0, 0.0)); \ + static_assert(0.0 == FUNC(0.0, -0.0)); \ + static_assert(0.0 == FUNC(-0.0, -0.0)); ---------------- These tests aren't covering what the sign of zero is in these cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134369/new/ https://reviews.llvm.org/D134369 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
