[libcxx-commits] [libcxx] WIP [libc++][math] Add `constexpr` to `copysign`, `copysignf`, `copysignl` (PR #216285)

[Aaron Ballman via libcxx-commits]

AaronBallman wrote:

> > > > The issue is that Clang doesn't make the C overloads `constexpr`, i.e. the `copysign`/`copysignf` etc. I'm quite certain that everything would work just fine if we change that.
> > > 
> > > 
> > > By that you mean Clang should just treat the non-`__builtin` version as `constexpr` even if it wasn’t declared `constexpr`? Do we do that for any other library functions?
> > 
> > 
> > I think the answer is: it's complicated. :-D
> > The C++ standard does not allow an implementation to add `constexpr` to library functions: https://eel.is/c++draft/constexpr.functions#1 and C does not have `constexpr` functions at all. So in theory, we're not allowed to do that in either language (though we can justify it more easily in C as defining the behavior for something which violates a constraint violation).
> > However, in practice, both Clang and GCC do it because that restriction leaves a lot of performance on the floor for somewhat questionable gains (it's observable whether something is or is not `constexpr` so this does harm portability, but I'm not certain users are all that worried about the portability harms). However, we don't do it consistently between the two compilers in that GCC accepts a lot more than Clang does and Clang doesn't diagnose our extension properly: https://godbolt.org/z/aorz9138K
> > I believe Clang's position has historically been that library functions follow the library definition requirements and the `__builtin` forms of library functions can be extended to support constant expressions. I think the behavior for `stdbit.h` may be anomalous in that regard. However, given that GCC supports this in both C and C++ on the non-builtin calls, I think there's a case to be made that Clang should follow suit (with appropriate pedantic diagnostics). However, I think this requires an RFC to get community buy-in. I think the main arguments for the RFC are that this makes it easier to write constant expressions without encouraging direct calls to builtin functions, the runtime performance might be better in some cases (I believe LLVM already knows how to constant fold basically all of this stuff and so performance gains may not be significant), and it improves compatibility with GCC. The argument against is that it is nonconforming (it can break conforming code given that C++ makes it possible to observe whether something is or is not a constant expression), so that should be explicitly discussed.
> 
> I spoke with someone from GCC off-list today and he wasn't certain whether the GCC behavior is intentional or not. I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127615 to see if we could get more details. If the behavior is intentional, then I think Clang should follow suit.

GCC reported back that this is by design but is more from the elder days of the compiler and not a recent change related to C++26 constexpr cmath support. e.g., https://godbolt.org/z/z9TzTq3KM

Here's my thinking:

1) Handle both `__builtin_foo` and `foo` forms of the call within Clang as constexpr library builtins. This means removing `OnlyBuiltinPrefixedAliasIsConstexpr` where appropriate and adding `Constexpr` to the list of attributes, and testing in both the old and new constexpr engines in both C and C++ language modes
2) libc++ is responsible for providing the `std::foo` implementation.
3) Diagnose the `foo` form in a constant expression evaluation context in C and C++ before C++26 as being a Clang (or GNU?) extension
4) Write an RFC to ensure the community is okay with this direction in C given that C has no constexpr function calls. In particular, it's worth pointing out that C++ has a rule that you're not allowed to add `constexpr` where it's not specified in the standard library and that WG14 could likely someday get a similar rule but does so before adding constexpr to all the math interfaces. GCC will be in a similar conundrum if that happens and hopefully it's not a deal breaker for anyone in our community, but I think that's a reason why we should have an RFC just to make sure people are comfortable with the direction.

What do others think?

https://github.com/llvm/llvm-project/pull/216285