RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696: not conformant features string

[Langer, Christoph]

Hi Lutz,

I also agree with Martin's idea of fixing this. I think it's better than changing existing feature strings just for the sake of quiescing a test.

I would even suggest to have a second Array of features in CPUInfo that contains all values in lowercase to be queried by CPUInfo::hasFeature. We should give it a try.

Best regards
Christoph

> -----Original Message-----
> From: hotspot-runtime-dev <hotspot-runtime-dev-
> bounces at openjdk.java.net> On Behalf Of Schmidt, Lutz
> Sent: Dienstag, 31. März 2020 15:59
> To: Doerr, Martin <martin.doerr at sap.com>; Thomas Stüfe
> <thomas.stuefe at gmail.com>
> Cc: hotspot-runtime-dev at openjdk.java.net
> Subject: [CAUTION] Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-
> 8238696: not conformant features string
> 
> Hi Martin,
> your extended check would heal the issue we saw with the original feature
> string on s390. So far, so good. I can’t judge if other tests can live with that.
> Regards,
> Lutz
> 
> From: "Doerr, Martin (martin.doerr at sap.com)" <martin.doerr at sap.com>
> Date: Tuesday, 31. March 2020 at 15:54
> To: Thomas Stüfe <thomas.stuefe at gmail.com>, Lutz Schmidt
> <lutz.schmidt at sap.com>
> Cc: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>, "hotspot-runtime-
> dev at openjdk.java.net" <hotspot-runtime-dev at openjdk.java.net>
> Subject: RE: RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696: not
> conformant features string
> 
> But what about
> features.contains(feature) || features.contains(feature.toLowerCase())
> ?
> 
> Is it still problematic if we just support both, lower case and exact match?
> 
> Best regards,
> Martin
> 
> 
> From: Thomas Stüfe <thomas.stuefe at gmail.com>
> Sent: Dienstag, 31. März 2020 15:51
> To: Schmidt, Lutz <lutz.schmidt at sap.com>
> Cc: Doerr, Martin <martin.doerr at sap.com>; Vladimir Ivanov
> <vladimir.x.ivanov at oracle.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696: not
> conformant features string
> 
> Okay. The change looks good to me.
> 
> Cheers, Thomas
> 
> On Tue, Mar 31, 2020 at 3:38 PM Schmidt, Lutz
> <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>> wrote:
> Thomas, Martin,
> 
> thank you for looking into this.
> 
> Generally, I'm very much in favor of the "fix the root cause" approach. In this
> particular case, however, I'm afraid of side effects and excessive effort.
> CPUInfo.hasFeature() is called in quite a few places, see the attached text
> file for reference.
> 
> My proposed change, on the other hand, is very constrained in scope: just
> s390, just a few string edits. The risk to break anything is extremely low.
> Therefore, I would like to stick with the proposed change.
> 
> Thanks,
> Lutz
> 
> 
> On 31.03.20, 11:33, "Doerr, Martin"
> <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
> 
>     Hi Thomas,
> 
>     I agree with that.
> 
>     test/lib/sun/hotspot/cpuinfo/CPUInfo.java uses
>     return features.contains(feature.toLowerCase());
>     in "hasFeature" which is not nicely designed.
> 
>     If it is required to lower the case (for some reason), I think it should better
> use
>     features.contains(feature) || features.contains(feature.toLowerCase())
> 
>     Or even better if we don't need toLowerCase at all and fix the strings.
> 
>     Anyway, the 2nd part of the change (SHA, AES, ...) looks good to me.
> 
>     Best regards,
>     Martin
> 
> 
>     > -----Original Message-----
>     > From: hotspot-runtime-dev <hotspot-runtime-dev-
>     > bounces at openjdk.java.net<mailto:bounces at openjdk.java.net>> On
> Behalf Of Thomas Stüfe
>     > Sent: Dienstag, 31. März 2020 10:01
>     > To: Schmidt, Lutz
> <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>>
>     > Cc: hotspot-runtime-dev at openjdk.java.net<mailto:hotspot-runtime-
> dev at openjdk.java.net>
>     > Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696:
> not
>     > conformant features string
>     >
>     > HI Lutz,
>     >
>     > Should we not better adjust the jtreg test?
>     >
>     > Cheers, Thomas
>     >
>     > On Tue, Mar 31, 2020 at 9:22 AM Schmidt, Lutz
> <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>>
>     > wrote:
>     >
>     > > Dear All,
>     > >
>     > > may I please request reviews for this fix, adjusting the CPU features
>     > > string such that jtreg tests, in particular a newly introduced one, pass
> OK.
>     > >
>     > > Bug:    https://bugs.openjdk.java.net/browse/JDK-8241101
>     > > Webrev: https://cr.openjdk.java.net/~lucy/webrevs/8241101.00/
>     > >
>     > > Thank you!
>     > > Lutz
>     > >
>     > >
>     > >
>     > >
>