RFR: JDK-8331732 : [PPC64] Unify and optimize code which converts != 0 to 1 [v4]
Martin Doerr
mdoerr at openjdk.org
Thu Jun 27 16:42:14 UTC 2024
On Thu, 27 Jun 2024 16:27:36 GMT, Suchismith Roy <sroy at openjdk.org> wrote:
>> [JDK-8331732](https://bugs.openjdk.org/browse/JDK-8331732)
>> The template interpreter contains branch-free conversion code for T_BOOLEAN (TemplateInterpreterGenerator::generate_result_handler_for).
>>
>> SharedRuntime::generate_native_wrapper uses unoptimized code to "Unpack the native result" for T_BOOLEAN.
>> Power10 has the "setbc" / "setbcr" instruction.
>>
>> A new function has been created for the conversion and use "setbcr" on Power10 (determined by VM_Version::has_brw()) and otherwise the branch-free implementation. We should have a function for 32 and one for 64 bit operations (or one with supports both).
>>
>> The new code for MacroAssembler::verify_secondary_supers_table also uses the new function.
>
> Suchismith Roy has updated the pull request incrementally with two additional commits since the last revision:
>
> - Comments and spaces
> - Comments and spaces
Thanks! This looks good now, except very minor nits.
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 27:
> 25:
> 26: #include "precompiled.hpp"
> 27: #include "asm/assembler.inline.hpp"
I think this is not needed because it is included via `macroAssembler.inline.hpp`.
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 351:
> 349:
> 350: if (VM_Version::has_brw()) {
> 351: if(is_64bit) {
Coding style should use a whitespace: `if (`
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 354:
> 352: cmpdi(CCR0, dst, 0);
> 353: }
> 354: else {
Coding style: `} else {`
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 359:
> 357: setbcr(dst, CCR0, Assembler::zero);
> 358: }
> 359: else {
Coding style: `} else {`
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 362:
> 360: neg(temp, dst);
> 361: orr(temp, dst, temp);
> 362: if(is_64bit) {
Coding style should use a whitespace: `if (`
-------------
PR Review: https://git.openjdk.org/jdk/pull/19886#pullrequestreview-2145925527
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1657457757
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1657449820
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1657450572
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1657452613
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1657451092
More information about the hotspot-dev
mailing list