The __has_constexpr_builtin macro can be used to check
whether the builtin in constant-evaluated by Clang frontend.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
A builtin is considered "constexpr" if it has E in its attributes in Builtins.def.
The list of constexpr builtins is consistent, the code in ExprConstant.cpp (where the actual constant evaluation of builtins is being done) guards it.
If builtin is not marked with E, it surely won't be evaluated, and vice versa - if builtin is marked but there is no code for evaluating it, there is a llvm_unreachable.
This macro will be needed because we are making constexpr <cmath> and <cstdlib>. We will conditionally make the functions constexpr until all supported compilers have full support.
We had a discussion where we found out that we need __has_builtin_constexpr: https://discourse.llvm.org/t/how-do-we-plan-to-make-constexpr-cmath-and-cstdlib/65930 Please read it if you wonder why we need it =) Thank you for your attention!
Comment Actions
Can you also add documentation and a release note for the new feature testing macro?
| clang/lib/Lex/PPMacroExpansion.cpp | ||
|---|---|---|
| 370 | It looks like unrelated formatting changes snuck in to this file. | |
| clang/test/Preprocessor/feature_tests.cpp | ||
| 56 | I'd appreciate some parsing tests: #if __has_constexpr_builtin
#endif
#if __has_constexpr_builtin("__builtin_fmax")
#endif
#if __has_constexpr_builtin(__builtin_fmax, __builtin_fmin)
#endif | |
Comment Actions
Please add some of this context to the release notes and/or documentation, thank you.
Comment Actions
The changes to the various Vist*CallExpr functions don't seem directly related to supporting __has_constexpr_builtin, can you explain the purpose of those changes? If they are not directly related maybe it makes more sense for them to be split off into a follow-up PR?
Comment Actions
Add more parsing tests. Fix unrelated formatting changes. Add release notes and docs.
| clang/lib/Lex/PPMacroExpansion.cpp | ||
|---|---|---|
| 370 | git clang-format HEAD~1 did it =) I reformatted unrelated changes back to original state. | |
Comment Actions
Hi! Added some text to the docs and to the release notes mentioning <cmath>. Please look at my wording =)
Hi! If I understood you correctly, every (or over 95%?) such function already has its tests checking its constant evaluation. For example test/Sema/constant-builtins-fmax.cpp checks constant __builtin_fmax.
We have a protocol that the E character in attributes marks that the builtin can be constant evaluated.
But the actual constant evaluation is done in lib/AST/ExprConstant.cpp and has little to do with builtin attributes.
So how do we keep the consistency of this protocol? In other words, how can we guarantee that IF the builtin is constant evaluated, it DOES have the E in Builtins.def?
I solved it with changes to Visit*CallExpr. Whenever we support constant evaluation in a builtin, we won't forget to mark its attributes with E (otherwise the evaluator returns early).
I was thinking to make a consistency test (outside the Clang code), but couldn't do it =(
| clang/docs/LanguageExtensions.rst | ||
|---|---|---|
| 96 ↗ | (On Diff #468314) | Just to be clear: this isn't a true statement now (std::fmax calls __builtin_fmax), but soon will be, after @philnik commits this patch https://reviews.llvm.org/D134584 part by part inline _LIBCPP_HIDE_FROM_ABI float fmax(float __x, float __y) _NOEXCEPT { return __builtin_fmaxf(__x, __y); } |
| clang/docs/LanguageExtensions.rst | ||
|---|---|---|
| 96 ↗ | (On Diff #468314) | BTW in future it could look something like this: inline _LIBCPP_CONSTEXPR_CXX23_IF_CONSTEXPR_BUILTIN(__builtin_fmax) _LIBCPP_HIDE_FROM_ABI float fmax(float __x, float __y) _NOEXCEPT { return __builtin_fmaxf(__x, __y); } |
Comment Actions
It looks like unrelated formatting changes snuck in to this file.
@aaron.ballman JFYI after I reverted what git clang-format HEAD~1 did to the code, the build has failed
ERROR git-clang-format returned an non-zero exit code 1 Build completed with failures
Can I apply git clang-format HEAD~1 again?
Comment Actions
That's our pre-commit CI reporting failures where there shouldn't be any. It's safe to ignore that failure, so there's no need to run clang-format again.
Comment Actions
LGTM, but please wait for @shafik before landing, in case he has more feedback or questions.
It looks like unrelated formatting changes snuck in to this file.