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

John R Rose jrose at openjdk.org
Tue Jan 31 21:58:48 UTC 2023


On Tue, 31 Jan 2023 20:05:21 GMT, Justin King <jcking at openjdk.org> wrote:

>> Deduplicate byte swapping implementations by consolidating them into `utilities/byteswap.hpp`, following `std::byteswap` introduced in C++23. Further simplification of `Bytes` will follow in https://github.com/openjdk/jdk/pull/12078.
>
> 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>

I would support folding compress/expand algorithms into moveBits.

Please manually check the assembly output of code_quality_reverse_bytes_32 and code_quality_reverse_bytes_64, as suggested by the comments, for each platform you test on, and confirm (with a comment on this PR) that the correct instruction is emitted.

I could suggest a microbenchmark (inside the gtest file) that would (with probable accuracy) detect failure of the intrinsic on some platform, by comparing a micro-loop that is suppose to compile with bswap to a micro-loop that uses an irregular ad hoc C expression of similar complexity but no known optimization.

Such a thing would bring in its own problems, including false positives (if run frequently, and not manually).  But if we were to do it right, perhaps imitating our jmh micro-benchmarks, it might add value, especially if we did the same with a whole raft of intrinsics (compress, multiply-high, etc. etc.).

But for the case of bswap I think we will certainly see regressions quickly, and quickly identify the root cause, so a micro-benchmark with pinpoint is not clearly warranted here.  IMO.

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

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


More information about the hotspot-jfr-dev mailing list