8268405: Several regressions 4-17% after CHA changes

Reviewed-by: kvn, dlong
This commit is contained in:
Vladimir Ivanov 2021-06-18 07:50:22 +00:00
parent 483f1ee211
commit 7ed3634da8
9 changed files with 61 additions and 41 deletions

View File

@ -767,6 +767,9 @@
"to stress handling of long counted loops: run inner loop" \
"for at most jint_max / StressLongCountedLoop") \
range(0, max_juint) \
\
product(bool, VerifyReceiverTypes, trueInDebug, DIAGNOSTIC, \
"Verify receiver types at runtime") \
// end of C2_FLAGS

View File

@ -515,6 +515,10 @@ bool LateInlineVirtualCallGenerator::do_late_inline_check(Compile* C, JVMState*
// Even if inlining is not allowed, a virtual call can be strength-reduced to a direct call.
bool allow_inline = C->inlining_incrementally();
if (!allow_inline && _callee->holder()->is_interface()) {
// Don't convert the interface call to a direct call guarded by an interface subtype check.
return false;
}
CallGenerator* cg = C->call_generator(_callee,
vtable_index(),
false /*call_does_dispatch*/,
@ -953,12 +957,12 @@ JVMState* PredictedCallGenerator::generate(JVMState* jvms) {
}
if (kit.stopped()) {
// Instance exactly does not matches the desired type.
// Instance does not match the predicted type.
kit.set_jvms(slow_jvms);
return kit.transfer_exceptions_into_jvms();
}
// fall through if the instance exactly matches the desired type
// Fall through if the instance matches the desired type.
kit.replace_in_map(receiver, casted_receiver);
// Make the hot call:

View File

@ -72,6 +72,9 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
Bytecodes::Code bytecode = caller->java_code_at_bci(bci);
guarantee(callee != NULL, "failed method resolution");
const bool is_virtual_or_interface = (bytecode == Bytecodes::_invokevirtual) ||
(bytecode == Bytecodes::_invokeinterface);
// Dtrace currently doesn't work unless all calls are vanilla
if (env()->dtrace_method_probes()) {
allow_inline = false;
@ -164,6 +167,18 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
bool should_delay = false;
if (ilt->ok_to_inline(callee, jvms, profile, should_delay)) {
CallGenerator* cg = CallGenerator::for_inline(callee, expected_uses);
// For optimized virtual calls assert at runtime that receiver object
// is a subtype of the inlined method holder. CHA can report a method
// as a unique target under an abstract method, but receiver type
// sometimes has a broader type. Similar scenario is possible with
// default methods when type system loses information about implemented
// interfaces.
if (cg != NULL && is_virtual_or_interface && !callee->is_static()) {
CallGenerator* trap_cg = CallGenerator::for_uncommon_trap(callee,
Deoptimization::Reason_receiver_constraint, Deoptimization::Action_none);
cg = CallGenerator::for_guarded_call(callee->holder(), trap_cg, cg);
}
if (cg != NULL) {
// Delay the inlining of this method to give us the
// opportunity to perform some high level optimizations
@ -344,9 +359,16 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
return CallGenerator::for_virtual_call(callee, vtable_index);
}
} else {
// Class Hierarchy Analysis or Type Profile reveals a unique target,
// or it is a static or special call.
return CallGenerator::for_direct_call(callee, should_delay_inlining(callee, jvms));
// Class Hierarchy Analysis or Type Profile reveals a unique target, or it is a static or special call.
CallGenerator* cg = CallGenerator::for_direct_call(callee, should_delay_inlining(callee, jvms));
// For optimized virtual calls assert at runtime that receiver object
// is a subtype of the method holder.
if (cg != NULL && is_virtual_or_interface && !callee->is_static()) {
CallGenerator* trap_cg = CallGenerator::for_uncommon_trap(callee,
Deoptimization::Reason_receiver_constraint, Deoptimization::Action_none);
cg = CallGenerator::for_guarded_call(callee->holder(), trap_cg, cg);
}
return cg;
}
}
@ -1105,12 +1127,12 @@ ciMethod* Compile::optimize_inlining(ciMethod* caller, ciInstanceKlass* klass, c
return NULL;
}
ciInstanceKlass *ikl = receiver_type->klass()->as_instance_klass();
if (ikl->is_loaded() && ikl->is_initialized() && !ikl->is_interface() &&
(ikl == actual_receiver || ikl->is_subtype_of(actual_receiver))) {
ciInstanceKlass* receiver_klass = receiver_type->klass()->as_instance_klass();
if (receiver_klass->is_loaded() && receiver_klass->is_initialized() && !receiver_klass->is_interface() &&
(receiver_klass == actual_receiver || receiver_klass->is_subtype_of(actual_receiver))) {
// ikl is a same or better type than the original actual_receiver,
// e.g. static receiver from bytecodes.
actual_receiver = ikl;
actual_receiver = receiver_klass;
// Is the actual_receiver exact?
actual_receiver_is_exact = receiver_type->klass_is_exact();
}
@ -1118,17 +1140,6 @@ ciMethod* Compile::optimize_inlining(ciMethod* caller, ciInstanceKlass* klass, c
ciInstanceKlass* calling_klass = caller->holder();
ciMethod* cha_monomorphic_target = callee->find_monomorphic_target(calling_klass, klass, actual_receiver, check_access);
// Validate receiver info against target method.
if (cha_monomorphic_target != NULL) {
bool has_receiver = !cha_monomorphic_target->is_static();
bool is_interface_holder = cha_monomorphic_target->holder()->is_interface();
if (has_receiver && !is_interface_holder) {
if (!cha_monomorphic_target->holder()->is_subtype_of(receiver_type->klass())) {
cha_monomorphic_target = NULL; // not a subtype
}
}
}
if (cha_monomorphic_target != NULL) {
// Hardwiring a virtual.
assert(!callee->can_be_statically_bound(), "should have been handled earlier");

View File

@ -2933,7 +2933,9 @@ Node* Phase::gen_subtype_check(Node* subklass, Node* superklass, Node** ctrl, No
}
Node* GraphKit::gen_subtype_check(Node* obj_or_subklass, Node* superklass) {
if (ExpandSubTypeCheckAtParseTime) {
bool expand_subtype_check = C->post_loop_opts_phase() || // macro node expansion is over
ExpandSubTypeCheckAtParseTime; // forced expansion
if (expand_subtype_check) {
MergeMemNode* mem = merged_memory();
Node* ctrl = control();
Node* subklass = obj_or_subklass;

View File

@ -1193,17 +1193,22 @@ void Parse::do_method_entry() {
make_dtrace_method_entry(method());
}
#ifdef ASSERT
// Narrow receiver type when it is too broad for the method being parsed.
ciInstanceKlass* callee_holder = method()->holder();
if (!method()->is_static()) {
ciInstanceKlass* callee_holder = method()->holder();
const Type* holder_type = TypeInstPtr::make(TypePtr::BotPTR, callee_holder);
Node* receiver_obj = local(0);
const TypeInstPtr* receiver_type = _gvn.type(receiver_obj)->isa_instptr();
if (receiver_type != NULL && !receiver_type->higher_equal(holder_type)) {
// Receiver should always be a subtype of callee holder.
// But, since C2 type system doesn't properly track interfaces,
// the invariant can't be expressed in the type system for default methods.
// Example: for unrelated C <: I and D <: I, (C `meet` D) = Object </: I.
assert(callee_holder->is_interface(), "missing subtype check");
#ifdef ASSERT
// Perform dynamic receiver subtype check against callee holder class w/ a halt on failure.
Node* holder_klass = _gvn.makecon(TypeKlassPtr::make(callee_holder));
Node* not_subtype_ctrl = gen_subtype_check(receiver_obj, holder_klass);
@ -1211,23 +1216,9 @@ void Parse::do_method_entry() {
Node* halt = _gvn.transform(new HaltNode(not_subtype_ctrl, frameptr(), "failed receiver subtype check"));
C->root()->add_req(halt);
#endif // ASSERT
// Receiver should always be a subtype of callee holder.
// But, since C2 type system doesn't properly track interfaces,
// the invariant on default methods can't be expressed in the type system.
// Example: for unrelated C <: I and D <: I, (C `meet` D) = Object </: I.
// (Downcasting interface receiver type to concrete class is fine, though it doesn't happen in practice.)
if (!callee_holder->is_interface()) {
assert(callee_holder->is_subtype_of(receiver_type->klass()), "sanity");
assert(!receiver_type->klass()->is_interface(), "interface receiver type");
receiver_type = receiver_type->join_speculative(holder_type)->is_instptr(); // keep speculative part
Node* casted_receiver_obj = _gvn.transform(new CheckCastPPNode(control(), receiver_obj, receiver_type));
set_local(0, casted_receiver_obj);
}
}
}
#endif // ASSERT
// If the method is synchronized, we need to construct a lock node, attach
// it to the Start node, and pin it there.

View File

@ -380,7 +380,7 @@ public:
Category category() const;
static const char* str(const Type* t);
#endif
#endif // !PRODUCT
void typerr(const Type *t) const; // Mixing types error
// Create basic type

View File

@ -1957,7 +1957,9 @@ JRT_ENTRY(void, Deoptimization::uncommon_trap_inner(JavaThread* current, jint tr
ScopeDesc* trap_scope = cvf->scope();
if (TraceDeoptimization) {
bool is_receiver_constraint_failure = VerifyReceiverTypes && (reason == Deoptimization::Reason_receiver_constraint);
if (TraceDeoptimization || is_receiver_constraint_failure) {
ttyLocker ttyl;
tty->print_cr(" bci=%d pc=" INTPTR_FORMAT ", relative_pc=" INTPTR_FORMAT ", method=%s" JVMCI_ONLY(", debug_id=%d"), trap_scope->bci(), p2i(fr.pc()), fr.pc() - nm->code_begin(), trap_scope->method()->name_and_sig_as_C_string()
#if INCLUDE_JVMCI
@ -2016,7 +2018,7 @@ JRT_ENTRY(void, Deoptimization::uncommon_trap_inner(JavaThread* current, jint tr
trap_method->name_and_sig_as_C_string(), trap_bci, nm->compiler_name());
// Print a bunch of diagnostics, if requested.
if (TraceDeoptimization || LogCompilation) {
if (TraceDeoptimization || LogCompilation || is_receiver_constraint_failure) {
ResourceMark rm;
ttyLocker ttyl;
char buf[100];
@ -2109,6 +2111,10 @@ JRT_ENTRY(void, Deoptimization::uncommon_trap_inner(JavaThread* current, jint tr
}
// (End diagnostic printout.)
if (is_receiver_constraint_failure) {
fatal("missing receiver type check");
}
// Load class if necessary
if (unloaded_class_index >= 0) {
constantPoolHandle constants(current, trap_method->constants());
@ -2598,6 +2604,7 @@ const char* Deoptimization::_trap_reason_name[] = {
"rtm_state_change",
"unstable_if",
"unstable_fused_if",
"receiver_constraint",
#if INCLUDE_JVMCI
"aliasing",
"transfer_to_interpreter",

View File

@ -89,6 +89,7 @@ class Deoptimization : AllStatic {
Reason_rtm_state_change, // rtm state change detected
Reason_unstable_if, // a branch predicted always false was taken
Reason_unstable_fused_if, // fused two ifs that had each one untaken branch. One is now taken.
Reason_receiver_constraint, // receiver subtype check failed
#if INCLUDE_JVMCI
Reason_aliasing, // optimistic assumption about aliasing failed
Reason_transfer_to_interpreter, // explicit transferToInterpreter()

View File

@ -2402,6 +2402,7 @@ typedef HashtableEntry<InstanceKlass*, mtClass> KlassHashtableEntry;
declare_constant(Deoptimization::Reason_rtm_state_change) \
declare_constant(Deoptimization::Reason_unstable_if) \
declare_constant(Deoptimization::Reason_unstable_fused_if) \
declare_constant(Deoptimization::Reason_receiver_constraint) \
NOT_ZERO(JVMCI_ONLY(declare_constant(Deoptimization::Reason_aliasing))) \
NOT_ZERO(JVMCI_ONLY(declare_constant(Deoptimization::Reason_transfer_to_interpreter))) \
NOT_ZERO(JVMCI_ONLY(declare_constant(Deoptimization::Reason_not_compiled_exception_handler))) \