review request (XXXL): 7023639: JSR 292 method handle invocation needs a fast path for compiled code

[John Rose]

On Jul 17, 2012, at 4:04 PM, Christian Thalinger wrote:

>> 
>> I see in several files next code pattern. Should we call throw_IncompatibleClassChangeError() as we do in other places?:
>> 
>> +  if (!EnableInvokeDynamic) {
>> +    // rewriter does not generate this bytecode
>> +    __ should_not_reach_here();
>> +    return;
>> +  }
> 
> Hmm.  This really should not happen and EnableInvokeDynamic is on by default anyway.  I doubt someone turns it off.

It's now a diagnostic switch.  It can be used to verify that an app. is not using 292, which is occasionally helpful.

>> constantPoolOop.cpp:
>> Why not use guarantee() for bad operants?
> 
> Not sure.  John?

The code in that file uses assert; I think I'm following the existing practice there.  The CP code operates after initial verification passes, so assert is suitable, I think.

>> Why you need separate scopes in resolve_bootstrap_specifier_at_impl()?
> 
> To keep oops from being visible.  Might result in bad crashes.

Yes.  An alternative is to have the x_oop variables visible at top level, but to put in DEBUG_ONLY(x_oop = NULL).  Having the temporary scoped seems cleaner to me, but maybe my sense of style is off here.

> 
> I will refresh the review patch in mlvm.  Thank you!

Yes, thanks!

— John