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

[Vladimir Kozlov]

Seems fine.

Vladimir

Christian Thalinger wrote:
> On May 12, 2011, at 7:34 PM, Vladimir Kozlov wrote:
>> ciMethodHandle constructor should initialize fields to some default values (it is C++, no automatic initialization).
> 
> Right.  I fixed that.
> 
>> Put caller_counter_data->count() value into a local var.
> 
> I talked to Tom about this and we came to the solution I wanted to do in the first place (but I thought was too complicated):  fake up the maturity of the MDO when the caller MDO is mature.
> 
> I also rolled in a last-minute fix from John:
> 
> http://cr.openjdk.java.net/~twisti/6998541/src/share/vm/prims/methodHandles.cpp.udiff.html
> 
> MethodHandles::init_AdapterMethodHandle needed a couple of additional ensure_vmlayout_field calls.  This was a crasher.
> 
> Sorry for asking again for a review.  Last time, I promise!  :-)
> 
> -- Christian
> 
>> Vladimir
>>
>> Christian Thalinger wrote:
>>> 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
> 
>