Request for reviews (L): 6939203: JSR 292 needs method handle constants

John Rose John.Rose at Sun.COM
Tue Apr 6 14:44:19 PDT 2010


On Apr 6, 2010, at 8:07 AM, Christian Thalinger wrote:

> On Tue, 2010-03-30 at 04:24 -0700, John Rose wrote:
>> 6939203: JSR 292 needs method handle constants
>> Summary: Add new CP types CONSTANT_MethodHandle, CONSTANT_MethodType; extend 'ldc' bytecode.
>>  http://cr.openjdk.java.net/~jrose/6939203/hs-webrev.00
> 
> src/share/vm/interpreter/bytecode.hpp:
> 
> 404   inline friend Bytecode_loadconstant* Bytecode_loadconstant_at(address bcp, methodHandle method);
> 405   inline friend Bytecode_loadconstant* Bytecode_loadconstant_at(methodHandle method, int bci);
> 
> Why are the arguments swapped?  int and address should be different
> types.

The (bcp, optional method) pattern mimics Bytecodes::code_at.
The (method, bci) pattern mimics a methodOop accessor.
The patterns don't mix well.  I'll try to boil this down to fewer overloadings.

> src/share/vm/interpreter/rewriter.cpp:
> 
> 142   Bytecodes::Code fastc = Bytecodes::_nop;
> 143   if (tag.is_method_handle() || tag.is_method_type()) {
> 144     fastc = (is_wide
> 145              ? Bytecodes::_fast_aldc_w
> 146              : Bytecodes::_fast_aldc);
> 147   }
> 148   if (fastc != Bytecodes::_nop) {
> 
> Why do you have a second if here?

No particular reason, other than leaving open an option to extend the aldc treatment to classes or other (future) constant types.  I'll merge the two blocks.

> src/share/vm/prims/jvm.h:
> 
> +    JVM_CONSTANT_MethodHandle           = 15,  // JSR 292
> +    JVM_CONSTANT_MethodType             = 16   // JSR 292
> 
> Is there a particular reason I don't see why this is 15 and 16?

Yes; I think the modules project is going to use the code point 13 and I wanted to leave a little extra room after it (14) in case they suddenly invent a second CP tag.  This will all look very funny in ten years.

>> Please review this small JDK change along with it:
>>  http://cr.openjdk.java.net/~jrose/6939203/jdk-webrev.00
> 
> Looks good.

Thanks!

-- John


More information about the hotspot-compiler-dev mailing list