archive/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2025-09-21 11:34:38 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

8005418: JSR 292: virtual dispatch bug in 292 impl

Browse source 
Reviewed-by: jrose, kvn
This commit is contained in:
Christian Thalinger 2013-01-09 15:37:23 -08:00
parent 38c81fb411
commit a55305503e
5 changed files with 102 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

22
hotspot/src/share/vm/opto/callGenerator.cpp
View file

@ -717,6 +717,7 @@ CallGenerator* CallGenerator::for_method_handle_call(JVMState* jvms, ciMethod* c
(input_not_const || !C->inlining_incrementally() || C->over_inlining_cutoff())) {
return CallGenerator::for_mh_late_inline(caller, callee, input_not_const);
} else {
// Out-of-line call.
return CallGenerator::for_direct_call(callee);
}
}
@ -787,9 +788,24 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
}
}
}
const int vtable_index = Method::invalid_vtable_index;
const bool call_is_virtual = target->is_abstract(); // FIXME workaround
CallGenerator* cg = C->call_generator(target, vtable_index, call_is_virtual, jvms, true, PROB_ALWAYS, true, true);
// Try to get the most accurate receiver type
const bool is_virtual = (iid == vmIntrinsics::_linkToVirtual);
const bool is_virtual_or_interface = (is_virtual || iid == vmIntrinsics::_linkToInterface);
int vtable_index = Method::invalid_vtable_index;
bool call_does_dispatch = false;
if (is_virtual_or_interface) {
ciInstanceKlass* klass = target->holder();
Node* receiver_node = kit.argument(0);
const TypeOopPtr* receiver_type = gvn.type(receiver_node)->isa_oopptr();
// call_does_dispatch and vtable_index are out-parameters. They might be changed.
target = C->optimize_virtual_call(caller, jvms->bci(), klass, target, receiver_type,
is_virtual,
call_does_dispatch, vtable_index); // out-parameters
}
CallGenerator* cg = C->call_generator(target, vtable_index, call_does_dispatch, jvms, true, PROB_ALWAYS, true, true);
assert(!cg->is_late_inline() || cg->is_mh_late_inline(), "no late inline here");
if (cg != NULL && cg->is_inline())
return cg;

11
hotspot/src/share/vm/opto/compile.hpp
View file

@ -72,6 +72,7 @@ class SafePointNode;
class JVMState;
class TypeData;
class TypePtr;
class TypeOopPtr;
class TypeFunc;
class Unique_Node_List;
class nmethod;
@ -740,9 +741,17 @@ class Compile : public Phase {
// Decide how to build a call.
// The profile factor is a discount to apply to this site's interp. profile.
CallGenerator* call_generator(ciMethod* call_method, int vtable_index, bool call_is_virtual, JVMState* jvms, bool allow_inline, float profile_factor, bool allow_intrinsics = true, bool delayed_forbidden = false);
CallGenerator* call_generator(ciMethod* call_method, int vtable_index, bool call_does_dispatch, JVMState* jvms, bool allow_inline, float profile_factor, bool allow_intrinsics = true, bool delayed_forbidden = false);
bool should_delay_inlining(ciMethod* call_method, JVMState* jvms);
// Helper functions to identify inlining potential at call-site
ciMethod* optimize_virtual_call(ciMethod* caller, int bci, ciInstanceKlass* klass,
ciMethod* callee, const TypeOopPtr* receiver_type,
bool is_virtual,
bool &call_does_dispatch, int &vtable_index);
ciMethod* optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
ciMethod* callee, const TypeOopPtr* receiver_type);
// Report if there were too many traps at a current method and bci.
// Report if a trap was recorded, and/or PerMethodTrapLimit was exceeded.
// If there is no MDO at all, report no trap unless told to assume it.

131
hotspot/src/share/vm/opto/doCall.cpp
View file

@ -61,7 +61,7 @@ void trace_type_profile(Compile* C, ciMethod *method, int depth, int bci, ciMeth
}
}
CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool call_is_virtual,
CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool call_does_dispatch,
JVMState* jvms, bool allow_inline,
float prof_factor, bool allow_intrinsics, bool delayed_forbidden) {
ciMethod* caller = jvms->method();
@ -82,7 +82,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
// See how many times this site has been invoked.
int site_count = profile.count();
int receiver_count = -1;
if (call_is_virtual && UseTypeProfile && profile.has_receiver(0)) {
if (call_does_dispatch && UseTypeProfile && profile.has_receiver(0)) {
// Receivers in the profile structure are ordered by call counts
// so that the most called (major) receiver is profile.receiver(0).
receiver_count = profile.receiver_count(0);
@ -94,7 +94,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
int r2id = (rid != -1 && profile.has_receiver(1))? log->identify(profile.receiver(1)):-1;
log->begin_elem("call method='%d' count='%d' prof_factor='%g'",
log->identify(callee), site_count, prof_factor);
if (call_is_virtual) log->print(" virtual='1'");
if (call_does_dispatch) log->print(" virtual='1'");
if (allow_inline) log->print(" inline='1'");
if (receiver_count >= 0) {
log->print(" receiver='%d' receiver_count='%d'", rid, receiver_count);
@ -111,12 +111,12 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
// We do this before the strict f.p. check below because the
// intrinsics handle strict f.p. correctly.
if (allow_inline && allow_intrinsics) {
CallGenerator* cg = find_intrinsic(callee, call_is_virtual);
CallGenerator* cg = find_intrinsic(callee, call_does_dispatch);
if (cg != NULL) {
if (cg->is_predicted()) {
// Code without intrinsic but, hopefully, inlined.
CallGenerator* inline_cg = this->call_generator(callee,
vtable_index, call_is_virtual, jvms, allow_inline, prof_factor, false);
vtable_index, call_does_dispatch, jvms, allow_inline, prof_factor, false);
if (inline_cg != NULL) {
cg = CallGenerator::for_predicted_intrinsic(cg, inline_cg);
}
@ -149,7 +149,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
float expected_uses = past_uses;
// Try inlining a bytecoded method:
if (!call_is_virtual) {
if (!call_does_dispatch) {
InlineTree* ilt;
if (UseOldInlining) {
ilt = InlineTree::find_subtree_from_root(this->ilt(), jvms->caller(), jvms->method());
@ -188,14 +188,14 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
} else if (require_inline || !InlineWarmCalls) {
return cg;
} else {
CallGenerator* cold_cg = call_generator(callee, vtable_index, call_is_virtual, jvms, false, prof_factor);
CallGenerator* cold_cg = call_generator(callee, vtable_index, call_does_dispatch, jvms, false, prof_factor);
return CallGenerator::for_warm_call(ci, cold_cg, cg);
}
}
}
// Try using the type profile.
if (call_is_virtual && site_count > 0 && receiver_count > 0) {
if (call_does_dispatch && site_count > 0 && receiver_count > 0) {
// The major receiver's count >= TypeProfileMajorReceiverPercent of site_count.
bool have_major_receiver = (100.*profile.receiver_prob(0) >= (float)TypeProfileMajorReceiverPercent);
ciMethod* receiver_method = NULL;
@ -209,7 +209,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
if (receiver_method != NULL) {
// The single majority receiver sufficiently outweighs the minority.
CallGenerator* hit_cg = this->call_generator(receiver_method,
vtable_index, !call_is_virtual, jvms, allow_inline, prof_factor);
vtable_index, !call_does_dispatch, jvms, allow_inline, prof_factor);
if (hit_cg != NULL) {
// Look up second receiver.
CallGenerator* next_hit_cg = NULL;
@ -219,7 +219,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
profile.receiver(1));
if (next_receiver_method != NULL) {
next_hit_cg = this->call_generator(next_receiver_method,
vtable_index, !call_is_virtual, jvms,
vtable_index, !call_does_dispatch, jvms,
allow_inline, prof_factor);
if (next_hit_cg != NULL && !next_hit_cg->is_inline() &&
have_major_receiver && UseOnlyInlinedBimorphic) {
@ -265,7 +265,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
// There was no special inlining tactic, or it bailed out.
// Use a more generic tactic, like a simple call.
if (call_is_virtual) {
if (call_does_dispatch) {
return CallGenerator::for_virtual_call(callee, vtable_index);
} else {
// Class Hierarchy Analysis or Type Profile reveals a unique target,
@ -397,6 +397,7 @@ void Parse::do_call() {
// orig_callee is the resolved callee which's signature includes the
// appendix argument.
const int nargs = orig_callee->arg_size();
const bool is_signature_polymorphic = MethodHandles::is_signature_polymorphic(orig_callee->intrinsic_id());
// Push appendix argument (MethodType, CallSite, etc.), if one.
if (iter().has_appendix()) {
@ -413,25 +414,18 @@ void Parse::do_call() {
// Then we may introduce a run-time check and inline on the path where it succeeds.
// The other path may uncommon_trap, check for another receiver, or do a v-call.
// Choose call strategy.
bool call_is_virtual = is_virtual_or_interface;
int vtable_index = Method::invalid_vtable_index;
ciMethod* callee = orig_callee;
// Try to get the most accurate receiver type
ciMethod* callee = orig_callee;
int vtable_index = Method::invalid_vtable_index;
bool call_does_dispatch = false;
if (is_virtual_or_interface) {
Node* receiver_node = stack(sp() - nargs);
const TypeOopPtr* receiver_type = _gvn.type(receiver_node)->isa_oopptr();
ciMethod* optimized_virtual_method = optimize_inlining(method(), bci(), klass, orig_callee, receiver_type);
// Have the call been sufficiently improved such that it is no longer a virtual?
if (optimized_virtual_method != NULL) {
callee = optimized_virtual_method;
call_is_virtual = false;
} else if (!UseInlineCaches && is_virtual && callee->is_loaded()) {
// We can make a vtable call at this site
vtable_index = callee->resolve_vtable_index(method()->holder(), klass);
}
// call_does_dispatch and vtable_index are out-parameters. They might be changed.
callee = C->optimize_virtual_call(method(), bci(), klass, orig_callee, receiver_type,
is_virtual,
call_does_dispatch, vtable_index); // out-parameters
}
// Note: It's OK to try to inline a virtual call.
@ -447,7 +441,7 @@ void Parse::do_call() {
// Decide call tactic.
// This call checks with CHA, the interpreter profile, intrinsics table, etc.
// It decides whether inlining is desirable or not.
CallGenerator* cg = C->call_generator(callee, vtable_index, call_is_virtual, jvms, try_inline, prof_factor());
CallGenerator* cg = C->call_generator(callee, vtable_index, call_does_dispatch, jvms, try_inline, prof_factor());
// NOTE: Don't use orig_callee and callee after this point! Use cg->method() instead.
orig_callee = callee = NULL;
@ -487,7 +481,7 @@ void Parse::do_call() {
// the call site, perhaps because it did not match a pattern the
// intrinsic was expecting to optimize. Should always be possible to
// get a normal java call that may inline in that case
cg = C->call_generator(cg->method(), vtable_index, call_is_virtual, jvms, try_inline, prof_factor(), /* allow_intrinsics= */ false);
cg = C->call_generator(cg->method(), vtable_index, call_does_dispatch, jvms, try_inline, prof_factor(), /* allow_intrinsics= */ false);
if ((new_jvms = cg->generate(jvms)) == NULL) {
guarantee(failing(), "call failed to generate: calls should work");
return;
@ -522,55 +516,44 @@ void Parse::do_call() {
round_double_result(cg->method());
ciType* rtype = cg->method()->return_type();
if (Bytecodes::has_optional_appendix(iter().cur_bc_raw())) {
// Be careful here with return types.
ciType* ctype = declared_signature->return_type();
if (Bytecodes::has_optional_appendix(iter().cur_bc_raw()) || is_signature_polymorphic) {
// Be careful here with return types.
if (ctype != rtype) {
BasicType rt = rtype->basic_type();
BasicType ct = ctype->basic_type();
Node* retnode = peek();
if (ct == T_VOID) {
// It's OK for a method to return a value that is discarded.
// The discarding does not require any special action from the caller.
// The Java code knows this, at VerifyType.isNullConversion.
pop_node(rt); // whatever it was, pop it
retnode = top();
} else if (rt == T_INT || is_subword_type(rt)) {
// FIXME: This logic should be factored out.
if (ct == T_BOOLEAN) {
retnode = _gvn.transform( new (C) AndINode(retnode, intcon(0x1)) );
} else if (ct == T_CHAR) {
retnode = _gvn.transform( new (C) AndINode(retnode, intcon(0xFFFF)) );
} else if (ct == T_BYTE) {
retnode = _gvn.transform( new (C) LShiftINode(retnode, intcon(24)) );
retnode = _gvn.transform( new (C) RShiftINode(retnode, intcon(24)) );
} else if (ct == T_SHORT) {
retnode = _gvn.transform( new (C) LShiftINode(retnode, intcon(16)) );
retnode = _gvn.transform( new (C) RShiftINode(retnode, intcon(16)) );
} else {
assert(ct == T_INT, err_msg_res("rt=%s, ct=%s", type2name(rt), type2name(ct)));
}
// Nothing. These cases are handled in lambda form bytecode.
assert(ct == T_INT || is_subword_type(ct), err_msg_res("must match: rt=%s, ct=%s", type2name(rt), type2name(ct)));
} else if (rt == T_OBJECT || rt == T_ARRAY) {
assert(ct == T_OBJECT || ct == T_ARRAY, err_msg_res("rt=%s, ct=%s", type2name(rt), type2name(ct)));
if (ctype->is_loaded()) {
const TypeOopPtr* arg_type = TypeOopPtr::make_from_klass(rtype->as_klass());
const Type* sig_type = TypeOopPtr::make_from_klass(ctype->as_klass());
if (arg_type != NULL && !arg_type->higher_equal(sig_type)) {
Node* retnode = pop();
Node* cast_obj = _gvn.transform(new (C) CheckCastPPNode(control(), retnode, sig_type));
pop();
push(cast_obj);
}
}
} else {
assert(ct == rt, err_msg("unexpected mismatch rt=%d, ct=%d", rt, ct));
assert(rt == ct, err_msg_res("unexpected mismatch: rt=%s, ct=%s", type2name(rt), type2name(ct)));
// push a zero; it's better than getting an oop/int mismatch
retnode = pop_node(rt);
retnode = zerocon(ct);
pop_node(rt);
Node* retnode = zerocon(ct);
push_node(ct, retnode);
}
// Now that the value is well-behaved, continue with the call-site type.
rtype = ctype;
}
} else {
assert(rtype == ctype, "mismatched return types"); // symbolic resolution enforces this
}
// If the return type of the method is not loaded, assert that the
@ -888,17 +871,39 @@ void Parse::count_compiled_calls(bool at_method_entry, bool is_inline) {
#endif //PRODUCT
ciMethod* Compile::optimize_virtual_call(ciMethod* caller, int bci, ciInstanceKlass* klass,
ciMethod* callee, const TypeOopPtr* receiver_type,
bool is_virtual,
bool& call_does_dispatch, int& vtable_index) {
// Set default values for out-parameters.
call_does_dispatch = true;
vtable_index = Method::invalid_vtable_index;
// Choose call strategy.
ciMethod* optimized_virtual_method = optimize_inlining(caller, bci, klass, callee, receiver_type);
// Have the call been sufficiently improved such that it is no longer a virtual?
if (optimized_virtual_method != NULL) {
callee = optimized_virtual_method;
call_does_dispatch = false;
} else if (!UseInlineCaches && is_virtual && callee->is_loaded()) {
// We can make a vtable call at this site
vtable_index = callee->resolve_vtable_index(caller->holder(), klass);
}
return callee;
}
// Identify possible target method and inlining style
ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
ciMethod *dest_method, const TypeOopPtr* receiver_type) {
ciMethod* Compile::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
ciMethod* callee, const TypeOopPtr* receiver_type) {
// only use for virtual or interface calls
// If it is obviously final, do not bother to call find_monomorphic_target,
// because the class hierarchy checks are not needed, and may fail due to
// incompletely loaded classes. Since we do our own class loading checks
// in this module, we may confidently bind to any method.
if (dest_method->can_be_statically_bound()) {
return dest_method;
if (callee->can_be_statically_bound()) {
return callee;
}
// Attempt to improve the receiver
@ -907,8 +912,8 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
if (receiver_type != NULL) {
// Array methods are all inherited from Object, and are monomorphic.
if (receiver_type->isa_aryptr() &&
dest_method->holder() == env()->Object_klass()) {
return dest_method;
callee->holder() == env()->Object_klass()) {
return callee;
}
// All other interesting cases are instance klasses.
@ -928,7 +933,7 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
}
ciInstanceKlass* calling_klass = caller->holder();
ciMethod* cha_monomorphic_target = dest_method->find_monomorphic_target(calling_klass, klass, actual_receiver);
ciMethod* cha_monomorphic_target = callee->find_monomorphic_target(calling_klass, klass, actual_receiver);
if (cha_monomorphic_target != NULL) {
assert(!cha_monomorphic_target->is_abstract(), "");
// Look at the method-receiver type. Does it add "too much information"?
@ -946,10 +951,10 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
cha_monomorphic_target->print();
tty->cr();
}
if (C->log() != NULL) {
C->log()->elem("missed_CHA_opportunity klass='%d' method='%d'",
C->log()->identify(klass),
C->log()->identify(cha_monomorphic_target));
if (log() != NULL) {
log()->elem("missed_CHA_opportunity klass='%d' method='%d'",
log()->identify(klass),
log()->identify(cha_monomorphic_target));
}
cha_monomorphic_target = NULL;
}
@ -961,7 +966,7 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
// by dynamic class loading. Be sure to test the "static" receiver
// dest_method here, as opposed to the actual receiver, which may
// falsely lead us to believe that the receiver is final or private.
C->dependencies()->assert_unique_concrete_method(actual_receiver, cha_monomorphic_target);
dependencies()->assert_unique_concrete_method(actual_receiver, cha_monomorphic_target);
return cha_monomorphic_target;
}
@ -970,7 +975,7 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
if (actual_receiver_is_exact) {
// In case of evolution, there is a dependence on every inlined method, since each
// such method can be changed when its class is redefined.
ciMethod* exact_method = dest_method->resolve_invoke(calling_klass, actual_receiver);
ciMethod* exact_method = callee->resolve_invoke(calling_klass, actual_receiver);
if (exact_method != NULL) {
#ifndef PRODUCT
if (PrintOpto) {

4
hotspot/src/share/vm/opto/parse.hpp
View file

@ -469,10 +469,6 @@ class Parse : public GraphKit {
// Helper function to uncommon-trap or bailout for non-compilable call-sites
bool can_not_compile_call_site(ciMethod *dest_method, ciInstanceKlass *klass);
// Helper function to identify inlining potential at call-site
ciMethod* optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
ciMethod *dest_method, const TypeOopPtr* receiver_type);
// Helper function to setup for type-profile based inlining
bool prepare_type_profile_inline(ciInstanceKlass* prof_klass, ciMethod* prof_method);

3
hotspot/src/share/vm/opto/parse1.cpp
View file

@ -1404,7 +1404,8 @@ void Parse::do_one_block() {
do_one_bytecode();
assert(!have_se || stopped() || failing() || (sp() - pre_bc_sp) == depth, "correct depth prediction");
assert(!have_se || stopped() || failing() || (sp() - pre_bc_sp) == depth,
err_msg_res("incorrect depth prediction: sp=%d, pre_bc_sp=%d, depth=%d", sp(), pre_bc_sp, depth));
do_exceptions();