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