Request for reviews (S): 7011839: JSR 292 turn on escape analysis when using invokedynamic

[Christian Thalinger]

On Jan 19, 2011, at 4:19 PM, Vladimir Kozlov wrote:
> Looks good.

Thank you, Vladimir.  -- Christian

> 
> Vladimir
> 
> On 1/19/11 2:26 AM, Christian Thalinger wrote:
>> Vladimir, could you please review this change?  -- Christian
>> 
>> On Jan 13, 2011, at 1:17 PM, Christian Thalinger wrote:
>>> On Jan 13, 2011, at 12:00 AM, John Rose wrote:
>>>> On Jan 12, 2011, at 5:46 AM, Christian Thalinger wrote:
>>>> 
>>>>> http://cr.openjdk.java.net/~twisti/7011839/webrev.01/
>>>> 
>>>> The arg_size computation is very fragile.  To start with, the arg_size value should be computed in one place.
>>>> 
>>>> As the code is written, arg_base points at the implicit argument to indy, which (I think) is not really on the stack.
>>>> 
>>>> I suggest this code:
>>>> 
>>>> 234   // compute size of arguments
>>>> 235   int arg_size = target->arg_size();
>>>>         if (code == Bytecodes::_invokedynamic) {
>>>>           assert(!target->is_static(), "receiver explicit in method");
>>>>           arg_size--;  // implicit, not really on stack
>>>>         }
>>>> 236   if (!target->is_loaded()&&  code == Bytecodes::_invokestatic) {
>>>>           assert(!target->is_static(), "receiver explicit in unloaded method");
>>>> 237     arg_size--;
>>>> 238   }
>>> 
>>> I like that better.  I'd like to keep the invokedynamic check as I couldn't reproduce any escaping case with an invokedynamic call site (using Indify).  I think the reason is because they all bail before out as native call sites.
>>> 
>>>> 
>>>> The second assertion is for good measure.  The bare "arg_size--" makes me nervous.
>>> 
>>> We can't use that assert since ciMethod::is_static uses check_is_loaded which asserts on is_loaded.
>>> 
>>> http://cr.openjdk.java.net/~twisti/7011839/webrev.02/