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

[Tom Rodriguez]

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
>