RFR: 8264173: [s390] Improve Hardware Feature Detection And Reporting

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8264173: [s390] Improve Hardware Feature Detection And Reporting

Lutz Schmidt
This enhancement is intended to improve the hardware feature detection and reporting, in particular for more recently introduced hardware. The enhancement is a prerequisite for possible future feature exploitation.

Reviews are highly welcome and appreciated.

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

Commit messages:
 - 8264173: [s390] Improve Hardware Feature Detection And Reporting

Changes: https://git.openjdk.java.net/jdk/pull/3196/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3196&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264173
  Stats: 549 lines in 4 files changed: 340 ins; 74 del; 135 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3196.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3196/head:pull/3196

PR: https://git.openjdk.java.net/jdk/pull/3196
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264173: [s390] Improve Hardware Feature Detection And Reporting [v2]

Lutz Schmidt
> This enhancement is intended to improve the hardware feature detection and reporting, in particular for more recently introduced hardware. The enhancement is a prerequisite for possible future feature exploitation.
>
> Reviews are highly welcome and appreciated.

Lutz Schmidt has updated the pull request incrementally with one additional commit since the last revision:

  update copyright headers

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3196/files
  - new: https://git.openjdk.java.net/jdk/pull/3196/files/3f524240..d15d1157

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3196&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3196&range=00-01

  Stats: 7 lines in 4 files changed: 0 ins; 0 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3196.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3196/head:pull/3196

PR: https://git.openjdk.java.net/jdk/pull/3196
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264173: [s390] Improve Hardware Feature Detection And Reporting [v2]

Martin Doerr
On Fri, 26 Mar 2021 14:07:52 GMT, Lutz Schmidt <[hidden email]> wrote:

>> This enhancement is intended to improve the hardware feature detection and reporting, in particular for more recently introduced hardware. The enhancement is a prerequisite for possible future feature exploitation.
>>
>> Reviews are highly welcome and appreciated.
>
> Lutz Schmidt has updated the pull request incrementally with one additional commit since the last revision:
>
>   update copyright headers

I didn't expect the change to become that large. But it looks good to me. The lengthy output only gets generated with -XX:+Verbose. That's fine.

src/hotspot/cpu/s390/vm_version_s390.cpp line 87:

> 85:                                    "system-z, g8-z14, ldisp_fast, extimm, pcrel_load/store, cmpb, cond_load/store, interlocked_update, txm, vectorinstr, instrext2, venh1)",
> 86:                                    "system-z, g9-z15, ldisp_fast, extimm, pcrel_load/store, cmpb, cond_load/store, interlocked_update, txm, vectorinstr, instrext2, venh1, instrext3, VEnh2 )"
> 87:                                   };

Would be nice to generate the feature string from a table it instead of having so many copies. But I'm ok with it for now.

src/hotspot/cpu/s390/vm_version_s390.cpp line 94:

> 92:
> 93:   if (Verbose || PrintAssembly || PrintStubCode) {
> 94:     print_features_internal("CPU Version as detected internally:  ", PrintAssembly || PrintStubCode);

2 spaces while other usages have no spaces?

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

Marked as reviewed by mdoerr (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3196
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264173: [s390] Improve Hardware Feature Detection And Reporting [v2]

Lutz Schmidt
On Mon, 29 Mar 2021 10:19:02 GMT, Martin Doerr <[hidden email]> wrote:

>> Lutz Schmidt has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   update copyright headers
>
> I didn't expect the change to become that large. But it looks good to me. The lengthy output only gets generated with -XX:+Verbose. That's fine.

Thank you for the review, Martin!

> src/hotspot/cpu/s390/vm_version_s390.cpp line 94:
>
>> 92:
>> 93:   if (Verbose || PrintAssembly || PrintStubCode) {
>> 94:     print_features_internal("CPU Version as detected internally:  ", PrintAssembly || PrintStubCode);
>
> 2 spaces while other usages have no spaces?

You are right. These spaces were excessive. Now they are gone.

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

PR: https://git.openjdk.java.net/jdk/pull/3196
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264173: [s390] Improve Hardware Feature Detection And Reporting [v2]

Goetz Lindenmaier
On Mon, 29 Mar 2021 10:32:40 GMT, Lutz Schmidt <[hidden email]> wrote:

>> I didn't expect the change to become that large. But it looks good to me. The lengthy output only gets generated with -XX:+Verbose. That's fine.
>
> Thank you for the review, Martin!

LGTM

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

PR: https://git.openjdk.java.net/jdk/pull/3196
Reply | Threaded
Open this post in threaded view
|

Integrated: 8264173: [s390] Improve Hardware Feature Detection And Reporting

Lutz Schmidt
In reply to this post by Lutz Schmidt
On Thu, 25 Mar 2021 15:28:21 GMT, Lutz Schmidt <[hidden email]> wrote:

> This enhancement is intended to improve the hardware feature detection and reporting, in particular for more recently introduced hardware. The enhancement is a prerequisite for possible future feature exploitation.
>
> Reviews are highly welcome and appreciated.

This pull request has now been integrated.

Changeset: d3fdd739
Author:    Lutz Schmidt <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/d3fdd739
Stats:     557 lines in 4 files changed: 340 ins; 74 del; 143 mod

8264173: [s390] Improve Hardware Feature Detection And Reporting

Reviewed-by: mdoerr, goetz

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

PR: https://git.openjdk.java.net/jdk/pull/3196