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

[Lindenmaier, Goetz]

Hi Fillipp, 

Oh, I understand.  Thanks for reviewing anyways!

Best regards,
  Goetz.

> -----Original Message-----
> From: Filipp Zhinkin [mailto:filipp.zhinkin at gmail.com]
> Sent: Donnerstag, 8. September 2016 16:14
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: Volker Simonis <volker.simonis at gmail.com>; hotspot-compiler-
> dev at openjdk.java.net
> Subject: Re: RFR(M): 8165235: [TESTBUG] RTM tests must check OS version
> 
> Hi Goetz,
> 
> sorry for the late reply.
> 
> The change looks good for me.
> Unfortunately I'm not able to sponsor it, because I'm not working at
> Oracle and can't submit JPRT.
> 
> Regards,
> Filipp.
> 
> On Tue, Sep 6, 2016 at 4:12 PM, Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com> wrote:
> > 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.