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

[Alexander Vorobyev]

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.

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).

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?

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
>>
>>
>>