[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron added a comment. In D134369#3843824 , @efriedma wrote: > You might need to explicitly specify -std=c++17 in the RUN line. (That bot > has a different target triple, and I think with that triple the default C++ > standard isn't C++17?) Thanks @e

[PATCH] D134369: [Clang] Support constexpr builtin fmax

efriedma added a comment. You might need to explicitly specify -std=c++17 in the RUN line. (That bot has a different target triple, and I think with that triple the default C++ standard isn't C++17?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron added a comment. I've got an email about "Buildbot failure" with this link: https://lab.llvm.org/buildbot/#/builders/216/builds/10955 Line 51: 'static_assert' with no message is a C++17 extension Why is it failing if pre-merge checks has been passed? =( Repository: rG LLVM Github M

[PATCH] D134369: [Clang] Support constexpr builtin fmax

This revision was automatically updated to reflect the committed changes. Closed by commit rG0edff6faa266: [Clang] Support constexpr builtin fmax (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134369/new/ https://reviews.llvm.or

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron updated this revision to Diff 466151. Izaron added a comment. Add TODO comment about sNaN. Thanks to @jcranmer-intel! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134369/new/ https://reviews.llvm.org/D134369 Files: clang/docs/LanguageExt

[PATCH] D134369: [Clang] Support constexpr builtin fmax

jcranmer-intel added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:14029-14030 + case Builtin::BI__builtin_fmaxf16: + case Builtin::BI__builtin_fmaxf128: { +APFloat RHS(0.); +if (!EvaluateFloat(E->getArg(0), Result, Info) || I think you sh

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron updated this revision to Diff 465106. Izaron added a comment. Fix comment wording. Thanks to @aaron.ballman! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134369/new/ https://reviews.llvm.org/D134369 Files: clang/docs/LanguageExtensions.

[PATCH] D134369: [Clang] Support constexpr builtin fmax

aaron.ballman added a comment. Regarding Annex F requirements, do we document that we conform to Annex F and do we have any actual test coverage for it? This question has come up for me quite a bit as I've worked on our C status page and I've never found a satisfactory answer of what we intend

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron updated this revision to Diff 464990. Izaron added a comment. Fix corner case with pos/neg zeroes Fix tests for pos/neg zeroes with __builtin_copysign Thanks to @efriedma and @jcranmer-intel! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D134369: [Clang] Support constexpr builtin fmax

efriedma added inline comments. 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)); \ +

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron updated this revision to Diff 463715. Izaron added a comment. Rebase and fix windows __float128 failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134369/new/ https://reviews.llvm.org/D134369 Files: clang/docs/LanguageExtensions.rst

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:14033-14034 +!EvaluateFloat(E->getArg(1), RHS, Info)) + return false; +if (Result.isNaN() || RHS > Result) + Result = RHS; jcranmer-intel wrote: > If I'm reading APFloa

[PATCH] D134369: [Clang] Support constexpr builtin fmax

efriedma added a comment. > We shouldn't be claiming that except maybe if we're using -fp-model=strict Makes sense. > I'm wondering what needs to happen with that for regular floating-point > operations that may trigger exceptions In general, floating-point exceptions are affected by whether w

[PATCH] D134369: [Clang] Support constexpr builtin fmax

jcranmer-intel added a comment. In 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

[PATCH] D134369: [Clang] Support constexpr builtin fmax

efriedma added reviewers: jcranmer-intel, john.brawn, fhahn, sepavloff. efriedma added a comment. 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

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron added a comment. This patch is derived from https://reviews.llvm.org/D134136 (which was abandoned due to poor tests and overcomplication) The constexpr version of `fmax` matches the libc realization: the libc realization is here

[PATCH] D134369: [Clang] Support constexpr builtin fmax

Izaron created this revision. Izaron added reviewers: efriedma, cor3ntin, aaron.ballman. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Support constexpr version of __builtin_fmax and its variations. R