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

John Rose john.r.rose at oracle.com
Thu Apr 22 00:08:35 PDT 2010


On Apr 6, 2010, at 2:44 PM, John Rose wrote:

> 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.

It boiled down nicely; I took out all but one overloading.  The ones based on the "address bcp" parameter were not actually useful.

I also added to interpreterRuntime.[ch]pp a bci(thread) accessor to use instead of the bcp(thread) accessor, and changed two unrelated places where bci(thread) was more obviously correct than bcp(thread).  Please look over those diffs.

>> 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.

Fixed.  The code is much simpler.

Here's the final webrev (if you agree that I fixed the issues you raised):
  http://cr.openjdk.java.net/~jrose/6939203/hs-webrev.01

-- John


More information about the hotspot-compiler-dev mailing list