RFR: JDK-8331732 : [PPC64] Unify and optimize code which converts != 0 to 1 [v12]

Amit Kumar amitkumar at openjdk.org
Fri Jun 28 15:39:22 UTC 2024


On Fri, 28 Jun 2024 14:43:33 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 one additional commit since the last revision:
> 
>   nits

Things you may consider: 
1. `is_64bit` also could be set to a default value; i.e. by default make it `true`; 
2.  I can see that `R0` is used temp register consistently, maybe make it default;

src/hotspot/cpu/ppc/assembler_ppc.hpp line 353:

> 351:     SETBC_OPCODE  = (31u << OPCODE_SHIFT | 384u << 1),
> 352:     SETNBC_OPCODE = (31u << OPCODE_SHIFT | 448u << 1),
> 353:     SETBCR_OPCODE = (31u << OPCODE_SHIFT | 416u << 1),

update copyright header.

src/hotspot/cpu/ppc/assembler_ppc.inline.hpp line 422:

> 420:   setnbc(d, bi0(cr, cc));
> 421: }
> 422: inline void Assembler::setbcr(Register d, int biint)

update copyright headers

src/hotspot/cpu/ppc/macroAssembler_ppc.hpp line 182:

> 180:   void inline set_cmpu3(Register dst, bool treat_unordered_like_less);
> 181:   // Branch-free implementation to convert !=0 to 1.
> 182:   void inline normalize_bool(Register dst, Register temp, bool use_64bit);

Suggestion:

  void inline normalize_bool(Register dst, Register temp, bool is_64bit);

src/hotspot/cpu/ppc/macroAssembler_ppc.inline.hpp line 268:

> 266: 
> 267: // Branch-free implementation to convert !=0 to 1
> 268: // Set register dst to 1 if dst is non-zero. Use setbcr instruction on Power10.

Suggestion:

// Set register dst to 1 if dst is non-zero. Uses setbcr instruction on Power10.

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

Changes requested by amitkumar (Committer).

PR Review: https://git.openjdk.org/jdk/pull/19886#pullrequestreview-2148294357
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1658922332
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1658919731
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1658912749
PR Review Comment: https://git.openjdk.org/jdk/pull/19886#discussion_r1658906625


More information about the hotspot-dev mailing list