Request for review: JDK-8145728: compiler/cpuflags/TestAESIntrinsicsOnSupportedConfig.java Expected message not found: 'com.sun.crypto.provider.AESCrypt::(implEncryptBlock|implDecryptBlock) ([0-9]+ bytes) (intrinsic) not found on supported platfroms

[Igor Ignatyev]

Alexander,

please see inline below.

> On Sep 30, 2016, at 4:51 PM, Alexander Vorobyev <alexander.vorobyev at oracle.com> wrote:
> 
> About my comment. I really was not able to reproduce this issue at that time, because the earliest failure report has different VM options and does not contain  -XX:TieredStopAtLevel option.
that means there can be another issue w/ the test or the product, which hasn’t investigated and your fix can hide it.
> 
> Do you always use -XX:TieredStopAtLevel option in test runs? My fix allows to run this test when this option is not set (vm.opt.TieredStopAtLevel == null).
no we don’t.
> 
> AESSupportPredicate class only checks CPU AES feature. It is exactly what it is supposed to do. Is it really necessary to add some new functionality (unrelated to AES feature) to it?
AESSupportPredicate is supposed to check that JVM can use AES, AFAIR there is AES intrinsics support only in C2, so a disabled C2 basically means JVM can not use AES intrinsics.

Regards,
— Igor
> 
> Thanks
> 
> On 30.09.2016 16:05, Igor Ignatyev wrote:
>> Alexander,
>> 
>> your fix literally removes the test from almost all executions, because we do not set -XX:TieredStopAtLevel=4 in any configs. from my point of view, changing AESSupportPredicate class is a better way to fix this issue, since it will be reused by all other tests.
>> 
>> I also have a question regarding your evaluation. Basing on own comment[1], not used C2 can not be a reason why this test failed before, otherwise you would be able to reproduce this bug w/o any problems. could you please provide more detailed evaluation?
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8145728?focusedCommentId=13996257&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13996257
>> It is not reproducible with the latest builds of JDK 9 (b133), even on the same host with the same options.
>> 
>> Thanks,
>> — Igor
>> 
>>> On Sep 29, 2016, at 7:30 PM, Alexander Vorobyev <alexander.vorobyev at oracle.com> wrote:
>>> 
>>> 
>>> Hi All,
>>> 
>>> I'd like review for JDK-8145728 (https://bugs.openjdk.java.net/browse/JDK-8145728)
>>> 
>>> Judging by the test results, test fails with specific compiler options: -XX:+TieredCompilation -XX:TieredStopAtLevel=N, where N<4. In this case C2 is not used and we are not able to see intrinsics usage in the test log. So such configuration is not valid for this test and should not be used. Supposed fix is to prevent this test from accepting such options.
>>> 
>>> "@requires" tag was added:
>>> @requires vm.opt.TieredStopAtLevel == null | vm.opt.TieredStopAtLevel == 4
>>> 
>>> 
>>> Here is webrev:
>>> http://cr.openjdk.java.net/~avorobye/8145728/webrew.00/
>>> 
>>> 
>>> Thanks,
>>> Alexander
>>> 
>>> 
>>> 
>