[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc14c34de4571: [clang][Interp] Implement __builtin_fmin (authored by tbaeder). Changed prior to commit:

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, we can handle NaN behavior in a follow-up. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155546/new/ https://reviews.llvm.org/D155546

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

aaron.ballman added a comment. In D155546#4541144 , @tbaeder wrote: > Thanks for the info @jcranmer-intel. > > I see that the current intepreter doesn't check this for `__builtin_fmin`(GCC > does) either, but it does error out for e.g.

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder added a comment. As for signaling vs. quiet NaNs, it seems like both GCC and Clang only check for NaNs, and don't care if it's signaling or not. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155546/new/ https://reviews.llvm.org/D155546

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder added a comment. Thanks for the info @jcranmer-intel. I see that the current intepreter doesn't check this for `__builtin_fmin`(GCC does) either, but it does error out for e.g. `__builtin_nan("") + 1` (GCC doesn't). @aaron.ballman I'd like to handle that in a separate patch where I

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

jcranmer-intel added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73 + constexpr float f1 = __builtin_fmin(1.0, 2.0f); + static_assert(f1 == 1.0f, ""); + + constexpr float min = __builtin_fmin(__builtin_nan(""), 1); + static_assert(min == 1,

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

jcranmer-intel added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73 + constexpr float f1 = __builtin_fmin(1.0, 2.0f); + static_assert(f1 == 1.0f, ""); + + constexpr float min = __builtin_fmin(__builtin_nan(""), 1); + static_assert(min == 1,

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

aaron.ballman added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73 + constexpr float f1 = __builtin_fmin(1.0, 2.0f); + static_assert(f1 == 1.0f, ""); + + constexpr float min = __builtin_fmin(__builtin_nan(""), 1); + static_assert(min == 1,

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73 + constexpr float f1 = __builtin_fmin(1.0, 2.0f); + static_assert(f1 == 1.0f, ""); + + constexpr float min = __builtin_fmin(__builtin_nan(""), 1); + static_assert(min == 1, ""); +

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

aaron.ballman added a comment. In D155546#4532839 , @jcranmer-intel wrote: > Treating sNaN as always signaling FE_INVALID is probably the safer option. That sounds reasonable to me. Comment at:

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder updated this revision to Diff 544691. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155546/new/ https://reviews.llvm.org/D155546 Files: clang/lib/AST/Interp/InterpBuiltin.cpp clang/test/AST/Interp/builtin-functions.cpp Index: clang/test/AST/Interp/builtin-functions.cpp

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

jcranmer-intel added a comment. In D155546#4510691 , @aaron.ballman wrote: > It's not yet clear to me what happens when any of these functions encounter a > signaling NaN at compile time. CC @hubert.reinterpretcast @jcranmer-intel > @rsmith

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

aaron.ballman added a comment. In D155546#4521383 , @tbaeder wrote: > In D155546#4510691 , @aaron.ballman > wrote: > >> I'd like to see test coverage for treatment of NaNs. According to the C23 >> standard, a

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder added a comment. In D155546#4510691 , @aaron.ballman wrote: > I'd like to see test coverage for treatment of NaNs. According to the C23 > standard, a quiet NaN is treated as missing data for fmax and fmin; so if > there's a quiet NaN and a

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

aaron.ballman added subscribers: jcranmer-intel, hubert.reinterpretcast, rsmith. aaron.ballman added a comment. I'd like to see test coverage for treatment of NaNs. According to the C23 standard, a quiet NaN is treated as missing data for fmax and fmin; so if there's a quiet NaN and a numeric

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder added a comment. `clang/test/Sema/constant-builtins-fmin.cpp` actually needed quite a lot of builtins before it could succeed, so it's enabled in https://reviews.llvm.org/D155369 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155546/new/

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

cor3ntin added a comment. Looks fine but where are the tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155546/new/ https://reviews.llvm.org/D155546 ___ cfe-commits mailing list

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

tbaeder created this revision. tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Split out from https://reviews.llvm.org/D155368