Skip to content

Commit

Permalink
8267807: C2: Downcast receiver to target holder during inlining
Browse files Browse the repository at this point in the history
Reviewed-by: roland, thartmann
  • Loading branch information
Vladimir Ivanov committed Jun 1, 2021
1 parent 1c7c0e1 commit 68f3b3a
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 43 deletions.
72 changes: 30 additions & 42 deletions src/hotspot/share/opto/doCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1097,59 +1097,47 @@ ciMethod* Compile::optimize_inlining(ciMethod* caller, ciInstanceKlass* klass, c
return callee;
}

if (receiver_type == NULL) {
return NULL; // no receiver type info
}

// Attempt to improve the receiver
bool actual_receiver_is_exact = false;
ciInstanceKlass* actual_receiver = klass;
if (receiver_type != NULL) {
// Array methods are all inherited from Object, and are monomorphic.
// finalize() call on array is not allowed.
if (receiver_type->isa_aryptr() &&
callee->holder() == env()->Object_klass() &&
callee->name() != ciSymbols::finalize_method_name()) {
return callee;
}
// Array methods are all inherited from Object, and are monomorphic.
// finalize() call on array is not allowed.
if (receiver_type->isa_aryptr() &&
callee->holder() == env()->Object_klass() &&
callee->name() != ciSymbols::finalize_method_name()) {
return callee;
}

// All other interesting cases are instance klasses.
if (!receiver_type->isa_instptr()) {
return NULL;
}
// All other interesting cases are instance klasses.
if (!receiver_type->isa_instptr()) {
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))) {
// ikl is a same or better type than the original actual_receiver,
// e.g. static receiver from bytecodes.
actual_receiver = ikl;
// Is the actual_receiver exact?
actual_receiver_is_exact = receiver_type->klass_is_exact();
}
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))) {
// ikl is a same or better type than the original actual_receiver,
// e.g. static receiver from bytecodes.
actual_receiver = ikl;
// 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) {
assert(!cha_monomorphic_target->is_abstract(), "");
// Look at the method-receiver type. Does it add "too much information"?
ciKlass* mr_klass = cha_monomorphic_target->holder();
const Type* mr_type = TypeInstPtr::make(TypePtr::BotPTR, mr_klass);
if (receiver_type == NULL || !receiver_type->higher_equal(mr_type)) {
// Calling this method would include an implicit cast to its holder.
// %%% Not yet implemented. Would throw minor asserts at present.
// %%% The most common wins are already gained by +UseUniqueSubclasses.
// To fix, put the higher_equal check at the call of this routine,
// and add a CheckCastPP to the receiver.
if (TraceDependencies) {
tty->print_cr("found unique CHA method, but could not cast up");
tty->print(" method = ");
cha_monomorphic_target->print();
tty->cr();
}
if (log() != NULL) {
log()->elem("missed_CHA_opportunity klass='%d' method='%d'",
log()->identify(klass),
log()->identify(cha_monomorphic_target));
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
}
cha_monomorphic_target = NULL;
}
}

Expand Down
39 changes: 38 additions & 1 deletion src/hotspot/share/opto/parse1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "opto/parse.hpp"
#include "opto/rootnode.hpp"
#include "opto/runtime.hpp"
#include "opto/type.hpp"
#include "runtime/handles.inline.hpp"
#include "runtime/safepointMechanism.hpp"
#include "runtime/sharedRuntime.hpp"
Expand Down Expand Up @@ -1192,6 +1193,42 @@ void Parse::do_method_entry() {
make_dtrace_method_entry(method());
}

// Narrow receiver type when it is too broad for the method being parsed.
ciInstanceKlass* callee_holder = method()->holder();
if (!method()->is_static()) {
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)) {

#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);
}

}
}

// If the method is synchronized, we need to construct a lock node, attach
// it to the Start node, and pin it there.
if (method()->is_synchronized()) {
Expand All @@ -1205,7 +1242,7 @@ void Parse::do_method_entry() {

// Setup Object Pointer
Node *lock_obj = NULL;
if(method()->is_static()) {
if (method()->is_static()) {
ciInstance* mirror = _method->holder()->java_mirror();
const TypeInstPtr *t_lock = TypeInstPtr::make(mirror);
lock_obj = makecon(t_lock);
Expand Down

1 comment on commit 68f3b3a

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.