Request for review (M): 6998541: JSR 292 implement missing return-type conversion for OP_RETYPE_RAW

Christian Thalinger christian.thalinger at oracle.com
Thu May 12 09:48:29 PDT 2011


On May 11, 2011, at 6:25 PM, Tom Rodriguez wrote:
> 
> On May 11, 2011, at 9:16 AM, Christian Thalinger wrote:
> 
>> On May 11, 2011, at 5:36 PM, Tom Rodriguez wrote:
>>> methodHandleWalk.cpp:
>>> 
>>> There are two copies of the retype code.  Can that be factored out?
>> 
>> Well, they are slightly different (change_argument vs. make_invoke) and the conversion for return types has still an Untested in there since I couldn't produce a test case to trigger that.
> 
> Oh I missed that difference.
> 
>> 
>> I can factor it with a bool for_return argument if that's better.
> 
> If it factors cleanly that would be nice.

Done.

> 
>> 
>>> 
>>> This doesn't seem right.  2 becomes false but 1 becomes true?
>>> 
>>> +         case T_BOOLEAN: {
>>> +           jvalue one_jvalue; one_jvalue.i = 1;
>>> +           ArgToken one = make_prim_constant(T_INT, &one_jvalue, CHECK_(zero));
>>> +           emit_load_constant(one);
>>> +           emit_bc(Bytecodes::_iand);
>>> +           break;
>>> +         }
>> 
>> I just asked John and that's how JSR 292 defines narrowing conversions to boolean.  That's how the interpreter does it and I mimic that in the compiler.
> 
> A little comment on it might be nice since it just seems so wrong.

Done.

While testing the code today I found two problems.

The first one was a wrong invoke instruction used for OP_PRIM_TO_REF (see methodHandleWalk.cpp:330).  ?.valueOf() is a static method and so we need to use an invokestatic.  I wonder how this ever worked before.

The other bug I did find while tracking down the first one:  when a method handle adapter does another MH invoke the profiling data is not available and the invoke count is -1.  I changed the code to pass the caller and call site bci into the MethodHandleCompiler so we can get the invoke count from the caller's MDO.

webrev is updated.

-- Christian

> 
> tom
> 
>> 
>>> 
>>> Put these on separate lines
>>> 
>>> !     Symbol* name; Symbol* sig;
>> 
>> Done.
>> 
>> -- Christian
>> 
>>> 
>>> Otherwise it looks good.
>>> 
>>> tom
>>> 
>>> 
>>> On May 11, 2011, at 3:42 AM, Christian Thalinger wrote:
>>> 
>>>> http://cr.openjdk.java.net/~twisti/6998541
>>>> 
>>>> 6998541: JSR 292 implement missing return-type conversion for OP_RETYPE_RAW
>>>> Reviewed-by: jrose
>>>> 
>>>> There is an unimplemented path in the MethodHandleWalker for
>>>> OP_RETYPE_RAW return-type conversions.
>>>> 
>>>> This change also includes a couple of x86 fixes found by John Rose,
>>>> removes the check for genericInvoker on x86 and SPARC and some
>>>> miscellaneous fixes (e.g. MethodHandlePrinter output).
>>>> 
>>>> There is also a test for the type conversions which will be pushed
>>>> later into the JDK 7 repository.
>>>> 
>>>> src/cpu/sparc/vm/methodHandles_sparc.cpp
>>>> src/cpu/x86/vm/methodHandles_x86.cpp
>>>> src/share/vm/prims/methodHandleWalk.cpp
>>>> src/share/vm/prims/methodHandleWalk.hpp
>>>> src/share/vm/prims/methodHandles.cpp
>>>> src/share/vm/prims/methodHandles.hpp




More information about the hotspot-compiler-dev mailing list