RFR(M): 8165235: [TESTBUG] RTM tests must check OS version

[Lindenmaier, Goetz]

Hi Filipp,

thanks for reviewing my change! 
I fixed the two issues:
http://cr.openjdk.java.net/~goetz/wr16/8165235-osRecog/03/webrev.bs/
http://cr.openjdk.java.net/~goetz/wr16/8165235-osRecog/03/webrev.hs/

The hotspot change is unchanged except for the reviewer attribution.

I also fixed the comment in Platform.java: major->minor.

Would you mind sponsoring the change?

Best regards,
  Goetz.


> -----Original Message-----
> From: Filipp Zhinkin [mailto:filipp.zhinkin at gmail.com]
> Sent: Dienstag, 6. September 2016 13:46
> To: Volker Simonis <volker.simonis at gmail.com>
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-compiler-
> dev at openjdk.java.net
> Subject: Re: RFR(M): 8165235: [TESTBUG] RTM tests must check OS version
> 
> Hi,
> 
> I would suggest to use something like Boolean.TRUE::booleanValue
> instead of null in AndPredicated ctor and use camel case for
> Platform's fields and methods.
> Otherwise the change looks good.
> 
> Just for the record: all those predicates where introduced because
> there were no way to check OS/CPU/whatever using jtreg.
> Now it should be possible to skip tests using jreg's @required tag. So
> maybe we can get rid of some java code? :)
> // Not suggesting to do it right now.
> 
> Regards,
> Filipp.
> 
> On Tue, Sep 6, 2016 at 1:21 PM, Volker Simonis <volker.simonis at gmail.com>
> wrote:
> > Thumbs up from me!
> >
> > Volker
> >
> > On Tue, Sep 6, 2016 at 11:11 AM, Lindenmaier, Goetz
> > <goetz.lindenmaier at sap.com> wrote:
> >> Hi Volker,
> >>
> >> thanks for the review! I fixed the two issues:
> >> http://cr.openjdk.java.net/~goetz/wr16/8165235-
> osRecog/02/webrev.hs/
> >> http://cr.openjdk.java.net/~goetz/wr16/8165235-
> osRecog/02/webrev.bs/
> >>
> >> Best regards,
> >>   Goetz.
> >>
> >>
> >>> -----Original Message-----
> >>> From: Volker Simonis [mailto:volker.simonis at gmail.com]
> >>> Sent: Montag, 5. September 2016 14:57
> >>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >>> Cc: hotspot-compiler-dev at openjdk.java.net
> >>> Subject: Re: RFR(M): 8165235: [TESTBUG] RTM tests must check OS
> version
> >>>
> >>> Hi Goetz,
> >>>
> >>> I think you've only forgot to import
> >>> compiler.testlibrary.rtm.predicate.SupportedOS into
> >>> test/compiler/rtm/cli/TestUseRTMLockingOptionWithBiasedLocking.java
> >>>
> >>> Also, in SupportedOS.java the line:
> >>>
> >>> public boolean getAsBoolean()
> >>>
> >>> is indented to far (should be four spaces less like the annotation in
> >>> the line before).
> >>>
> >>> Besides that, the change looks good.
> >>>
> >>> Thanks for fixing this,
> >>> Volker
> >>>
> >>> On Mon, Sep 5, 2016 at 1:54 PM, Lindenmaier, Goetz
> >>> <goetz.lindenmaier at sap.com> wrote:
> >>> > Hi,
> >>> >
> >>> >
> >>> >
> >>> > This fixes the RTM tests wrt. to supported platforms on ppc.
> >>> >
> >>> > Please review this change. I please need a sponsor.
> >>> > http://cr.openjdk.java.net/~goetz/wr16/8165235-
> osRecog/01/webrev.bs/
> >>> >
> >>> > http://cr.openjdk.java.net/~goetz/wr16/8165235-
> osRecog/01/webrev.hs/
> >>> >
> >>> >
> >>> > RTM uses special instructions that are only available on recent x86
> cpus. On
> >>> > x86, this feature does not need OS support. On ppc, the equivalent
> >>> > functionality, hardware transactional memory, requires OS support.
> Thus
> >>> the
> >>> > feature is only enabled by the VM if CPU and OS are at a specific level.
> The
> >>> > tests must check this. too. This holds for AIX and Linux.
> >>> >
> >>> >
> >>> >
> >>> > To do so, this change introduces rtm/predicate/SupportedOS.java
> which
> >>> checks
> >>> > for proper OS versions on ppc, else returns true.
> >>> >
> >>> > The OS version is retrieved from Platform.java, which has new
> methods
> >>> > getOsVersionMajor() and  getOsVersionMinor().
> >>> >
> >>> > To simplify the checks in the tests, I also introduced a 3-way
> AndPredicate
> >>> > constructor.
> >>> >
> >>> >
> >>> >
> >>> > To simplify the OS version check on Aix, I change enabling RTM on Aix
> to
> >>> > require AIX 7.2.
> >>> >
> >>> > Before, it was enabled on AIX 7.1.3.30, which contains an important
> bug fix.
> >>> > The
> >>> >
> >>> > last digits of this version are not exported to os.version property, so I
> >>> > can not
> >>> >
> >>> > check for them in the test.
> >>> >
> >>> >
> >>> >
> >>> > Best regards,
> >>> >
> >>> >   Goetz.