RFR: JDK-8300783: Consolidate byteswap implementations [v13]

[Justin King]

On Wed, 15 Feb 2023 10:59:58 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Justin King has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix copyright
>>   
>>   Signed-off-by: Justin King <jcking at google.com>
>
> src/hotspot/share/utilities/byteswap.hpp line 55:
> 
>> 53: 
>> 54: template <typename T, ENABLE_IF(CanByteswapImpl<T>::value)>
>> 55: ALWAYSINLINE T byteswap(T x) {
> 
> Can this be constexpr?  I think it probably can, assuming all the various intrinsics are constexpr.
> 
> Also, we don't generally use ALWAYSINLINE unless there is a demonstrated reason to do so.  There are some 
> operator new's that use it to help with the allocation recording.  The oop_oop_iterate suite uses it because
> they were found to often not inline, and ensuring they were inlined was found to be performance critical.
> Other than that, there are a handful of uses, some of which might not actually be needed.

`__builtin_bswapX` and `__builtin_bitreverseX` where not always `constexpr` compatible in Clang or GCC, though both will usually generate compile time constants in `-O2` if given compile time constants as inputs. That support for use with `constexpr` was only added in the last couple of versions in preparation for C++20. To be able to be `constexpr` compatible, while using intrinsics ensuring we get the best generated code, we would have to replicate something like `std::is_constant_evaluated()` from C++20 and `__has_constexpr_builtin`.

However no existing usages in Hotspot rely on them being `constexpr` now, so it should be fine.

I replaced all uses of `ALWAYSINLINE` with just `inline`, until we can prove that `ALWAYSINLINE` is needed as suggested.

> src/hotspot/share/utilities/moveBits.hpp line 52:
> 
>> 50: 
>> 51: template <typename T, ENABLE_IF(CanReverseBitsImpl<T>::value)>
>> 52: ALWAYSINLINE T reverse_bits(T x) {
> 
> So far as I know, ALWAYSINLINE does not imply constexpr.  So this is a feature regression.

Same as above, no existing usages rely on this being `constexpr`. So we have to choose between ensuring the best generated code vs. `constexpr`.

-------------

PR: https://git.openjdk.org/jdk/pull/12114