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]

On 30.09.2016 16:58, Igor Ignatyev wrote:
> 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.
I don't think my fix can hide it, because it does not use VM 
options/configurations from the earliest failure reports. Only issue my 
fix is targeted for is invalid VM configuration with 
-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).
> 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.
Maybe I was wrong. For now,  AESSupportPredicate uses CPUInfo 
class which shows us exactly CPU features, not JVM. And, for example, 
TestAESIntrinsicsOnUnsupportedConfig.java expects exactly such 
behaviour. Because "UnsupportedConfig" means CPU with no AES feature. On 
such CPU we will see "AES instructions are not available on this CPU" 
warning (TestAESIntrinsicsOnUnsupportedConfig expects this warning) in 
the test log, but on CPU with AES feature and with 
-XX:TieredStopAtLevel=1 option (you suppose to make AESSupportPredicate 
return FALSE for  this configuration, right?) we won't. In result, we 
will have TestAESIntrinsicsOnUnsupportedConfig failures on platforms 
where this test is not even supposed to be run. Please correct me if I 
misunderstand your idea.
>
> 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
>>>>
>>>>
>>>>