[13] RFR(S): 8218201: Failures when vmIntrinsics::_getClass is not inlined

[dean.long at oracle.com]

+1

dl

On 3/8/19 10:29 AM, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 3/8/19 12:25 AM, Tobias Hartmann wrote:
>> Vladimir, Dean, thanks for the review.
>>
>> My first fix version had '_return_allocated = false' but then I 
>> incorrectly assumed that the
>> behavior would be the same as if we don't special case at all. I 
>> missed that the receiver is of
>> course also involved and you are right that we should not treat it as 
>> "global escape".
>>
>> New webrev:
>> http://cr.openjdk.java.net/~thartmann/8218201/webrev.01/
>>
>> Best regards,
>> Tobias
>>
>> On 07.03.19 20:20, dean.long at oracle.com wrote:
>>> I agree.  We don't want obj.getClass() to treat obj as "global escape".
>>>
>>> dl
>>>
>>> PS - Original change goes back to JDK-6488063.
>>>
>>> On 3/7/19 10:46 AM, Vladimir Kozlov wrote:
>>>> _getClass is special case because it is native call and we can't do 
>>>> bytecode analysis.
>>>> But we know what it does (it loads klass from object and mirror 
>>>> from klass) - no allocations, no
>>>> locals, no arguments returned.
>>>> I think it is simple missing _return_allocated = false setting in 
>>>> addition to _return_local =
>>>> false. _return_allocated is set to true by default optimistically.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 3/7/19 4:54 AM, Tobias Hartmann wrote:
>>>>> Hi,
>>>>>
>>>>> please review the following patch:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8218201
>>>>> http://cr.openjdk.java.net/~thartmann/8218201/webrev.00/
>>>>>
>>>>> When intrinsification is disabled, the BCEscapeAnalyzer marks the 
>>>>> return value of the (native)
>>>>> method Object::getClass as "return allocated value" which means 
>>>>> that "only newly allocated unescaped
>>>>> objects are returned". The OptimizePtrCompare optimization then 
>>>>> uses this information to incorrectly
>>>>> fold 'obj.getClass() == Object.class' (see TestGetClass.java:39) 
>>>>> to always false.
>>>>>
>>>>> This is a very old issue and I can't trace back why a special case 
>>>>> for the _getClass intrinsic has
>>>>> been added to the BCEscapeAnalyzer. Since I don't think we should 
>>>>> make any assumptions about the
>>>>> returned Object, I've removed the special case.
>>>>>
>>>>> Thanks,
>>>>> Tobias
>>>>>
>>>