RFR(XS): 8213410: UseCompressedOops @requirement check fails fails on 32-bit system

[Boris Ulasevich]

Hi David,

Thank you.

Don't you think it is better to learn jtreg to perform short-circuit 
boolean evaluation?

http://cr.openjdk.java.net/~bulasevich/8213410/webrev.02
http://cr.openjdk.java.net/~bulasevich/8213410/webrev.02_jtreg

By the way, with the given change in jtreg two failed tests from the 
original list works Ok because they already have 64-bit check on 
previous lines:
* @requires (sun.arch.data.model == "64")
* @requires vm.opt.final.UseCompressedOops

regards,
Boris

On 09.11.2018 10:39, David Holmes wrote:
> Hi Boris,
> 
> On 7/11/2018 6:54 PM, David Holmes wrote:
>> Hi Boris,
>>
>> On 7/11/2018 6:06 PM, Boris Ulasevich wrote:
>>> Hi David,
>>>
>>>    Yes, at first glance it is weird. We have actually three states 
>>> for vm.opt.final.UseCompressedOops: true, false and null. Null means 
>>> "not applicable" - when current VM does not support the option with 
>>> the given name. Here is another approach for the issue (I am not sure 
>>> it is a good one):
>>>    http://cr.openjdk.java.net/~bulasevich/8213410/webrev.01/
>>
>> I like that approach! I'm just not sure everyone else will necessarily 
>> agree :) This proposal might need a wider audience than just hotspot-dev.
>>
>> Let me make enquiries with jtreg-use at openjdk.java.net and see what the 
>> general intent was here.
> 
> That was uninformative as to intent as Jon initially thought both 
> variants should fail the same way, but on further reflection it is 
> likely due to the difference between trying to evaluate null as a 
> boolean - which gives an error - and doing "true".equals(null) , which 
> is just false.
> 
> I also see now that this isn't a general facility for using
> 
> @requires <some VM flag>
> 
> because we only actually support three directly:
> 
>   protected void vmOptFinalFlags(Map<String, String> map) {
>          vmOptFinalFlag(map, "ClassUnloading");
>          vmOptFinalFlag(map, "UseCompressedOops");
>          vmOptFinalFlag(map, "EnableJVMCI");
>    }
> 
> but that also shows this is not specific to UseCompressedOops because 
> EnableJVMCI only exists in a VM that was built with JVMCI support.
> 
> I still think the correct way for this current case to be handled in 
> tests is by using:
> 
> @requires vm.bits == 64 & UseCompressedOops
> 
> though that requires jtreg to do short-circuit boolean evaluation, which 
> it appears not to do. :( Though:
> 
> @requires vm.bits == 64 & UseCompressedOops == true
> 
> should work (albeit perhaps due to implementation accident).
> 
> Cheers,
> David
> -----
> 
>> Aside: if the UseCompressOops checks is preceded by a 64-bit check 
>> does it still fail? (Just wondering whether jtreg does short-circuit 
>> evaluation for these @require statements.)
>>
>> Thanks,
>> David
>>
>>> thank you,
>>> Boris
>>>
>>> On 07.11.2018 1:36, David Holmes wrote:
>>>> Hi Boris,
>>>>
>>>> I'm somewhat perplexed as to what the difference is between:
>>>>
>>>> @requires vm.opt.final.UseCompressedOops
>>>>
>>>> and
>>>>
>>>> @requires vm.opt.final.UseCompressedOops == true
>>>>
>>>> if they are indeed different then that seems to be a bug in the 
>>>> requirement handling in the jtreg support code.
>>>>
>>>> I'm also wondering whether any such requirement should always be 
>>>> proceeded by a check for 64-bits?
>>>>
>>>> Otherwise I would expect
>>>>
>>>> @requires vm.opt.final.UseCompressedOops
>>>>
>>>> to be false in 32-bit, so again a problem with the jtreg support code.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 7/11/2018 12:43 AM, Boris Ulasevich wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this patch to fix jtreg @requires 
>>>>> vm.opt.final.UseCompressedOops flag evaluation fail reproduced on 
>>>>> ARM32: "invalid boolean value: null for expression 
>>>>> vm.opt.final.UseCompressedOops".
>>>>>
>>>>> http://cr.openjdk.java.net/~bulasevich/8213410/webrev.00/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8213410
>>>>>
>>>>> The fix was checked on ARM32 (tests disabled) and AARCH64 (works Ok 
>>>>> by default, and becomes disabled with 
>>>>> -javaoptions:"-XX:-UseCompressedOops" jtreg option).
>>>>>
>>>>> Thanks,
>>>>> Boris Ulasevich