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

[Filipp Zhinkin]

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.