review request (XXXL): 7023639: JSR 292 method handle invocation needs a fast path for compiled code
Christian Thalinger
christian.thalinger at oracle.com
Thu Jul 19 15:10:04 PDT 2012
We forgot to remove the Ricochet Frame code from the sA:
http://cr.openjdk.java.net/~twisti/7023639/
-- Chris
On Jul 19, 2012, at 11:28 AM, Christian Thalinger wrote:
> JDK testing found a small bug:
>
> diff --git a/src/share/vm/oops/cpCacheOop.cpp b/src/share/vm/oops/cpCacheOop.cpp
> --- a/src/share/vm/oops/cpCacheOop.cpp
> +++ b/src/share/vm/oops/cpCacheOop.cpp
> @@ -486,7 +486,8 @@
> // virtual and final so _f2 contains method ptr instead of vtable index
> if (f2_as_vfinal_method() == old_method) {
> // match old_method so need an update
> - set_f2_as_vfinal_method(new_method);
> + // NOTE: can't use set_f2_as_vfinal_method as it asserts on different values
> + _f2 = (intptr_t)new_method;
> if (RC_TRACE_IN_RANGE(0x00100000, 0x00400000)) {
> if (!(*trace_name_printed)) {
> // RC_TRACE_MESG macro has an embedded ResourceMark
>
> I will integrate this one.
>
> -- Chris
>
> On Jul 18, 2012, at 7:10 PM, John Rose wrote:
>
>> On Jul 17, 2012, at 4:04 PM, Christian Thalinger wrote:
>>
>>>>
>>>> I see in several files next code pattern. Should we call throw_IncompatibleClassChangeError() as we do in other places?:
>>>>
>>>> + if (!EnableInvokeDynamic) {
>>>> + // rewriter does not generate this bytecode
>>>> + __ should_not_reach_here();
>>>> + return;
>>>> + }
>>>
>>> Hmm. This really should not happen and EnableInvokeDynamic is on by default anyway. I doubt someone turns it off.
>>
>> It's now a diagnostic switch. It can be used to verify that an app. is not using 292, which is occasionally helpful.
>>
>>>> constantPoolOop.cpp:
>>>> Why not use guarantee() for bad operants?
>>>
>>> Not sure. John?
>>
>> The code in that file uses assert; I think I'm following the existing practice there. The CP code operates after initial verification passes, so assert is suitable, I think.
>>
>>>> Why you need separate scopes in resolve_bootstrap_specifier_at_impl()?
>>>
>>> To keep oops from being visible. Might result in bad crashes.
>>
>> Yes. An alternative is to have the x_oop variables visible at top level, but to put in DEBUG_ONLY(x_oop = NULL). Having the temporary scoped seems cleaner to me, but maybe my sense of style is off here.
>>
>>>
>>> I will refresh the review patch in mlvm. Thank you!
>>
>> Yes, thanks!
>>
>> — John
>> _______________________________________________
>> mlvm-dev mailing list
>> mlvm-dev at openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
More information about the mlvm-dev
mailing list