RFR(L) 8223320: [AOT] jck test api/javax_script/ScriptEngine/PutGet.html fails when test classes are AOTed

[Igor Veresov]

Thanks Vladimir and Tom!


igor



> On May 31, 2019, at 12:36 PM, Tom Rodriguez <tom.rodriguez at oracle.com> wrote:
> 
> Looks good.
> 
> tom
> 
> Igor Veresov wrote on 5/31/19 12:30 PM:
>> Thanks for review!
>> I did the renames and added support for ByteCache.
>> Updated webrev: http://cr.openjdk.java.net/~iveresov/8223320/webrev.02/ <http://cr.openjdk.java.net/%7Eiveresov/8223320/webrev.02/>
>> igor
>>> On May 31, 2019, at 11:36 AM, Tom Rodriguez <tom.rodriguez at oracle.com <mailto:tom.rodriguez at oracle.com>> wrote:
>>> 
>>> you're missing the ByteCache.
>>> 
>>> Isn't box the generic term for these classes whether they are cached or not?  I guess what’s confusing is that both new Integer(n) and Integer.valueOf(n) are both boxes but one with be marked as box and the other won't.  It might be clearer to use cached or auto_box instead of just box.
>>> 
>>> Otherwise this looks ok to me.  Thanks for taking care of this.
>>> 
>>> tom
>>> 
>>> Igor Veresov wrote on 5/30/19 3:29 PM:
>>>>> On May 30, 2019, at 2:15 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com> <mailto:vladimir.kozlov at oracle.com>> wrote:
>>>>> 
>>>>> CCing to runtime group too.
>>>>> 
>>>>> So you went hard and correct way ;-)
>>>> Well, there isn’t much of choice… Must abide the spec.
>>>>> 
>>>>> deoptimization.cpp next #ifdef sequence is strange since we support only 64-bit on SPARC:
>>>>> 
>>>>> +#ifdef _LP64
>>>>> +                       jlong res = (jlong)low->get_int();
>>>>> +#else
>>>>> +#ifdef SPARC
>>>>> 
>>>> Right. I guess JVMCI and AOT will never support 32 bit, so I’ll just remove it.
>>>>> Also the code here is guarded by INCLUDE_JVMCI but it should be applicable to AOT code too. Right? May be || INCLUDE_AOT?
>>>>> 
>>>> Good point, will add that.
>>>>> Please, add more comment. For example add one in aotLoader.cpp for initialize_box_caches() to explain why we need to eager initialize caches for AOT.
>>>>> 
>>>> Ok.
>>>> New webrev: http://cr.openjdk.java.net/~iveresov/8223320/webrev.01/ <http://cr.openjdk.java.net/%7Eiveresov/8223320/webrev.01/> <http://cr.openjdk.java.net/%7Eiveresov/8223320/webrev.01/>
>>>> igor
>>>>> Thanks,
>>>>> Vladimir
>>>>> 
>>>>> On 5/30/19 12:33 PM, Igor Veresov wrote:
>>>>>> Graal models boxing (a call to valueOf()) as a BoxNode. If scalarized, it is encoded in the debug info as an allocation of a box object. However, for certain ranges of values the box object has to come from caches. The reason is that for these values JLS guarantees the identity of the boxes.
>>>>>> The fix essentially propagates the information on whether the Box is a result of Box.valueOf() or new Box() to the deoptimization machinery that checks if the object is in the range that should be in a cache and gets it from there instead of allocating it.
>>>>>> Mach5: tier1-6, tier2-6 with Graal
>>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8223320/webrev.00/ <http://cr.openjdk.java.net/%7Eiveresov/8223320/webrev.00/> <http://cr.openjdk.java.net/%7Eiveresov/8223320/webrev.00/>
>>>>>> I’d like to push all this into JDK13 first and then follow up with a change to the upstream Graal.
>>>>>> Thanks,
>>>>>> igor

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20190531/247a9eb4/attachment.html>