Skip to content

Commit

Permalink
8268405: Several regressions 4-17% after CHA changes
Browse files Browse the repository at this point in the history
Reviewed-by: kvn, dlong
  • Loading branch information
Vladimir Ivanov committed Jun 18, 2021
1 parent 483f1ee commit 7ed3634
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 41 deletions.
3 changes: 3 additions & 0 deletions src/hotspot/share/opto/c2_globals.hpp
Expand Up @@ -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

Expand Down
8 changes: 6 additions & 2 deletions src/hotspot/share/opto/callGenerator.cpp
Expand Up @@ -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*/,
Expand Down Expand Up @@ -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:
Expand Down
47 changes: 29 additions & 18 deletions src/hotspot/share/opto/doCall.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -1105,30 +1127,19 @@ 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();
}

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");
Expand Down
4 changes: 3 additions & 1 deletion src/hotspot/share/opto/graphKit.cpp
Expand Up @@ -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;
Expand Down
25 changes: 8 additions & 17 deletions src/hotspot/share/opto/parse1.cpp
Expand Up @@ -1193,41 +1193,32 @@ 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);
assert(!stopped(), "not a subtype");

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.
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/type.hpp
Expand Up @@ -380,7 +380,7 @@ class Type {
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
Expand Down
11 changes: 9 additions & 2 deletions src/hotspot/share/runtime/deoptimization.cpp
Expand Up @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/deoptimization.hpp
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/vmStructs.cpp
Expand Up @@ -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))) \
Expand Down

0 comments on commit 7ed3634

Please sign in to comment.