Request for reviews (M): 6910618: C2: Error: assert(d->is_oop(),"JVM_ArrayCopy: dst not an oop")

[Tom Rodriguez]

s/guaranty/guaranteed/  otherwise that looks good.

tom

On Feb 8, 2010, at 4:28 PM, Vladimir Kozlov wrote:

> http://cr.openjdk.java.net/~kvn/6910618/webrev.01
> 
> I added comment about return_oop in deoptimization.cpp.
> And I added the regression test case. I finally made it fail
> with current VM (have to force inlining StringBuilder::append).
> 
> Thanks,
> Vladimir
> 
> Tom Rodriguez wrote:
>> On Feb 8, 2010, at 1:44 PM, Vladimir Kozlov wrote:
>>> Thank you, Tom
>>> 
>>> Bytecode_invoke_at_check() does not check for allocation bytecodes
>>> and the bug case is deoptimization on the return from the call to
>>> OptoRuntime::new_array_C(). So I am nervous about checking only
>>> bytecode since I may miss something.
>> I hadn't even considered that case but your fix definitely seems right.  You might put a larger comment on return_oop indicating under what conditions it's used any what it can't be divined from the bytecodes themselves.
>> tom
>>> I will remove printing.
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> Tom Rodriguez wrote:
>>>> Oh, right, wrong end of the stack.  So then why can't you get it from the bytecodes of the call site of the last scope?
>>>> ScopeDesc* scope = chunk->at(chunk->length() - 1)->scope();
>>>> Bytecode_invoke* invoke = Bytecode_invoke_at_check(scope->method(), scope->bci());
>>>> if (invoke != NULL) {
>>>> save_oop_result = invoke->result_type() == T_OBJECT;
>>>> }
>>>> I guess it's possible that the site is being handled in some different way such that we don't actually have a real call there, particularly if reexecute were true.  So your fix seems more right.  What do you think about dropping the =true on the printing output since it seems redundant?
>>>> tom
>>>> On Feb 8, 2010, at 11:50 AM, Vladimir Kozlov wrote:
>>>>> Because it will return deoptee (caller method) and not the method
>>>>> from which we returned already (no frame on stack).
>>>>> 
>>>>> Vladimir
>>>>> 
>>>>> Tom Rodriguez wrote:
>>>>>> Why do you need to record this in the PcDesc?  Why can't you just do:
>>>>>> chunk->at(0)->scope()->method()->return_type() == T_OBJECT
>>>>>> tom
>>>>>> On Feb 8, 2010, at 11:12 AM, Vladimir Kozlov wrote:
>>>>>>> http://cr.openjdk.java.net/~kvn/6910618/webrev.00
>>>>>>> 
>>>>>>> Fixed 6910618: C2: Error: assert(d->is_oop(),"JVM_ArrayCopy: dst not an oop")
>>>>>>> 
>>>>>>> Problem:
>>>>>>> If deoptimization happened on the return from a call which
>>>>>>> which returns oop, the oop will be not updated during GC which
>>>>>>> is triggered by scalar replaced objects reallocation.
>>>>>>> 
>>>>>>> Solution:
>>>>>>> Mark in PcDesc call sites which return oop (this is main part of changes)
>>>>>>> and save the result oop across objects reallocation during deoptimization.
>>>>>>> 
>>>>>>> Reviewed by:
>>>>>>> 
>>>>>>> Fix verified (y/n): y, test
>>>>>>> 
>>>>>>> Other testing:
>>>>>>> JPRT
>>>>>>>