RFR: 8333382: [s390x] Enhance popcnt Instruction to use Z15 facilities [v4]

Lutz Schmidt lucy at openjdk.org
Mon Jun 10 13:10:15 UTC 2024


On Sat, 8 Jun 2024 14:06:30 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

>> We need to move popcnt instruction implementation out of s390.ad file as it is required to be required some methods present in [JDK-8331126.](https://bugs.openjdk.org/browse/JDK-8331126)
>> 
>> 
>> When the miscellaneous-instruction-extensions facility 3 is not installed or bit 0 of the M3
>>  field is zero, a count of the number of one bits in each of the eight bytes of general register
>>  R2 is placed into the corresponding byte of general register R1. Each byte of general register 
>> R1 is an 8-bit binary integer in the range of 0-8.
>> 
>> 
>> 
>> When the miscellaneous-instruction-extensions facility 3 is installed and bit 0 of the M3 field
>> is one, a count of the total number of one bits in the 64-bit general register R2 is placed into 
>> general register R1. The result is a 64-bit unsigned integer in the range 0 to 64.
>> 
>> 
>> Performed tier1 test on fastdebug build and didn't see any regression.
>
> Amit Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   removes warmup code, changes time unit and iteration number

I like it much better now (aka LGTM).
Would be nice if you would remove the comments I complained about.

src/hotspot/cpu/s390/s390.ad line 10691:

> 10689:     Register Rsrc = $src$$Register;
> 10690: 
> 10691:     // Prefer compile-time assertion over run-time SIGILL.

This comment isn't very helpful anymore.

src/hotspot/cpu/s390/s390.ad line 10711:

> 10709:     Register Rsrc = $src$$Register;
> 10710: 
> 10711:     // Prefer compile-time assertion over run-time SIGILL.

This comment isn't very helpful anymore.

src/hotspot/cpu/s390/s390.ad line 10731:

> 10729:     Register Rtmp = $tmp$$Register;
> 10730: 
> 10731:     // Prefer compile-time assertion over run-time SIGILL.

This comment isn't very helpful anymore.

src/hotspot/cpu/s390/s390.ad line 10752:

> 10750:     Register Rtmp = $tmp$$Register;
> 10751: 
> 10752:     // Prefer compile-time assertion over run-time SIGILL.

This comment isn't very helpful anymore.

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

Marked as reviewed by lucy (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19509#pullrequestreview-2107741714
PR Review Comment: https://git.openjdk.org/jdk/pull/19509#discussion_r1633215587
PR Review Comment: https://git.openjdk.org/jdk/pull/19509#discussion_r1633216081
PR Review Comment: https://git.openjdk.org/jdk/pull/19509#discussion_r1633217583
PR Review Comment: https://git.openjdk.org/jdk/pull/19509#discussion_r1633218248


More information about the hotspot-dev mailing list