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

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu May 12 13:41:30 PDT 2011

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 callercounterdata->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::initAdapterMethodHandle needed a couple of additional ensurevmlayoutfield 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 (changeargument vs. makeinvoke) 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 forreturn 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 TBOOLEAN: { + jvalue onejvalue; onejvalue.i = 1; + ArgToken one = makeprimconstant(TINT, &onejvalue, CHECK(zero)); + emitloadconstant(one); + emitbc(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 OPPRIMTOREF (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 OPRETYPERAW Reviewed-by: jrose There is an unimplemented path in the MethodHandleWalker for OPRETYPERAW 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/methodHandlessparc.cpp src/cpu/x86/vm/methodHandlesx86.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