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

[Filipp Zhinkin]

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.