Skip to content

Commit

Permalink
6312651: Compiler should only use verified interface types for optimi…
Browse files Browse the repository at this point in the history
…zation

Reviewed-by: vlivanov, kvn
  • Loading branch information
rwestrel committed Nov 21, 2022
1 parent bcc6b12 commit 45d1807
Show file tree
Hide file tree
Showing 20 changed files with 1,101 additions and 842 deletions.
4 changes: 3 additions & 1 deletion src/hotspot/share/ci/ciArrayKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
#include "ci/ciArrayKlass.hpp"
#include "ci/ciObjArrayKlass.hpp"
#include "ci/ciTypeArrayKlass.hpp"
#include "ci/ciUtilities.hpp"
#include "ci/ciUtilities.inline.hpp"
#include "memory/universe.hpp"

// ciArrayKlass
//
Expand Down Expand Up @@ -103,3 +104,4 @@ ciArrayKlass* ciArrayKlass::make(ciType* element_type) {
return ciObjArrayKlass::make(element_type->as_klass());
}
}

1 change: 1 addition & 0 deletions src/hotspot/share/ci/ciArrayKlass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class ciArrayKlass : public ciKlass {
bool is_array_klass() const { return true; }

static ciArrayKlass* make(ciType* element_type);

};

#endif // SHARE_CI_CIARRAYKLASS_HPP
26 changes: 26 additions & 0 deletions src/hotspot/share/ci/ciInstanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ ciInstanceKlass::ciInstanceKlass(Klass* k) :
_nonstatic_fields = NULL; // initialized lazily by compute_nonstatic_fields:
_has_injected_fields = -1;
_implementor = NULL; // we will fill these lazily
_transitive_interfaces = NULL;

// Ensure that the metadata wrapped by the ciMetadata is kept alive by GC.
// This is primarily useful for metadata which is considered as weak roots
Expand Down Expand Up @@ -729,6 +730,31 @@ void ciInstanceKlass::dump_replay_instanceKlass(outputStream* out, InstanceKlass
}
}

GrowableArray<ciInstanceKlass*>* ciInstanceKlass::transitive_interfaces() const{
if (_transitive_interfaces == NULL) {
const_cast<ciInstanceKlass*>(this)->compute_transitive_interfaces();
}
return _transitive_interfaces;
}

void ciInstanceKlass::compute_transitive_interfaces() {
GUARDED_VM_ENTRY(
InstanceKlass* ik = get_instanceKlass();
Array<InstanceKlass*>* interfaces = ik->transitive_interfaces();
Arena* arena = CURRENT_ENV->arena();
int len = interfaces->length() + (is_interface() ? 1 : 0);
GrowableArray<ciInstanceKlass*>* transitive_interfaces = new(arena)GrowableArray<ciInstanceKlass*>(arena, len,
0, NULL);
for (int i = 0; i < interfaces->length(); i++) {
transitive_interfaces->append(CURRENT_ENV->get_instance_klass(interfaces->at(i)));
}
if (is_interface()) {
transitive_interfaces->append(this);
}
_transitive_interfaces = transitive_interfaces;
);
}

void ciInstanceKlass::dump_replay_data(outputStream* out) {
ResourceMark rm;

Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/ci/ciInstanceKlass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,11 @@ class ciInstanceKlass : public ciKlass {
// A ciInstanceKlass that's not itself: one implementor.
// Itself: more than one implementor.
ciInstanceKlass* _implementor;
GrowableArray<ciInstanceKlass*>* _transitive_interfaces;

void compute_injected_fields();
bool compute_injected_fields_helper();
void compute_transitive_interfaces();

protected:
ciInstanceKlass(Klass* k);
Expand Down Expand Up @@ -292,6 +294,7 @@ class ciInstanceKlass : public ciKlass {
bool has_trusted_loader() const {
return _has_trusted_loader;
}
GrowableArray<ciInstanceKlass*>* transitive_interfaces() const;

// Replay support

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/ci/ciObjectFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ void ciObjectFactory::init_shared_objects() {
assert (obj->is_metadata(), "what else would it be?");
if (obj->is_loaded() && obj->is_instance_klass()) {
obj->as_instance_klass()->compute_nonstatic_fields();
obj->as_instance_klass()->transitive_interfaces();
}
}
}
Expand Down
80 changes: 10 additions & 70 deletions src/hotspot/share/opto/castnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,83 +403,23 @@ const Type* CheckCastPPNode::Value(PhaseGVN* phase) const {
const Type *inn = phase->type(in(1));
if( inn == Type::TOP ) return Type::TOP; // No information yet

const TypePtr *in_type = inn->isa_ptr();
const TypePtr *my_type = _type->isa_ptr();
if (inn->isa_oopptr() && _type->isa_oopptr()) {
return ConstraintCastNode::Value(phase);
}

const TypePtr *in_type = inn->isa_ptr();
const TypePtr *my_type = _type->isa_ptr();
const Type *result = _type;
if( in_type != NULL && my_type != NULL ) {
TypePtr::PTR in_ptr = in_type->ptr();
if (in_type != NULL && my_type != NULL) {
TypePtr::PTR in_ptr = in_type->ptr();
if (in_ptr == TypePtr::Null) {
result = in_type;
} else if (in_ptr == TypePtr::Constant) {
if (my_type->isa_rawptr()) {
result = my_type;
} else {
const TypeOopPtr *jptr = my_type->isa_oopptr();
assert(jptr, "");
result = !in_type->higher_equal(_type)
? my_type->cast_to_ptr_type(TypePtr::NotNull)
: in_type;
}
} else {
result = my_type->cast_to_ptr_type( my_type->join_ptr(in_ptr) );
result = my_type->cast_to_ptr_type(my_type->join_ptr(in_ptr));
}
}

// This is the code from TypePtr::xmeet() that prevents us from
// having 2 ways to represent the same type. We have to replicate it
// here because we don't go through meet/join.
if (result->remove_speculative() == result->speculative()) {
result = result->remove_speculative();
}

// Same as above: because we don't go through meet/join, remove the
// speculative type if we know we won't use it.
return result->cleanup_speculative();

// JOIN NOT DONE HERE BECAUSE OF INTERFACE ISSUES.
// FIX THIS (DO THE JOIN) WHEN UNION TYPES APPEAR!

//
// Remove this code after overnight run indicates no performance
// loss from not performing JOIN at CheckCastPPNode
//
// const TypeInstPtr *in_oop = in->isa_instptr();
// const TypeInstPtr *my_oop = _type->isa_instptr();
// // If either input is an 'interface', return destination type
// assert (in_oop == NULL || in_oop->klass() != NULL, "");
// assert (my_oop == NULL || my_oop->klass() != NULL, "");
// if( (in_oop && in_oop->klass()->is_interface())
// ||(my_oop && my_oop->klass()->is_interface()) ) {
// TypePtr::PTR in_ptr = in->isa_ptr() ? in->is_ptr()->_ptr : TypePtr::BotPTR;
// // Preserve cast away nullness for interfaces
// if( in_ptr == TypePtr::NotNull && my_oop && my_oop->_ptr == TypePtr::BotPTR ) {
// return my_oop->cast_to_ptr_type(TypePtr::NotNull);
// }
// return _type;
// }
//
// // Neither the input nor the destination type is an interface,
//
// // history: JOIN used to cause weird corner case bugs
// // return (in == TypeOopPtr::NULL_PTR) ? in : _type;
// // JOIN picks up NotNull in common instance-of/check-cast idioms, both oops.
// // JOIN does not preserve NotNull in other cases, e.g. RawPtr vs InstPtr
// const Type *join = in->join(_type);
// // Check if join preserved NotNull'ness for pointers
// if( join->isa_ptr() && _type->isa_ptr() ) {
// TypePtr::PTR join_ptr = join->is_ptr()->_ptr;
// TypePtr::PTR type_ptr = _type->is_ptr()->_ptr;
// // If there isn't any NotNull'ness to preserve
// // OR if join preserved NotNull'ness then return it
// if( type_ptr == TypePtr::BotPTR || type_ptr == TypePtr::Null ||
// join_ptr == TypePtr::NotNull || join_ptr == TypePtr::Constant ) {
// return join;
// }
// // ELSE return same old type as before
// return _type;
// }
// // Not joining two pointers
// return join;
return result;
}

//=============================================================================
Expand Down
83 changes: 3 additions & 80 deletions src/hotspot/share/opto/cfgnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1180,42 +1180,12 @@ const Type* PhiNode::Value(PhaseGVN* phase) const {
}
}

// Until we have harmony between classes and interfaces in the type
// lattice, we must tread carefully around phis which implicitly
// convert the one to the other.
const TypePtr* ttp = _type->make_ptr();
const TypeInstPtr* ttip = (ttp != NULL) ? ttp->isa_instptr() : NULL;
const TypeInstKlassPtr* ttkp = (ttp != NULL) ? ttp->isa_instklassptr() : NULL;
bool is_intf = false;
if (ttip != NULL) {
if (ttip->is_interface())
is_intf = true;
}
if (ttkp != NULL) {
if (ttkp->is_interface())
is_intf = true;
}

// Default case: merge all inputs
const Type *t = Type::TOP; // Merged type starting value
for (uint i = 1; i < req(); ++i) {// For all paths in
// Reachable control path?
if (r->in(i) && phase->type(r->in(i)) == Type::CONTROL) {
const Type* ti = phase->type(in(i));
// We assume that each input of an interface-valued Phi is a true
// subtype of that interface. This might not be true of the meet
// of all the input types. The lattice is not distributive in
// such cases. Ward off asserts in type.cpp by refusing to do
// meets between interfaces and proper classes.
const TypePtr* tip = ti->make_ptr();
const TypeInstPtr* tiip = (tip != NULL) ? tip->isa_instptr() : NULL;
if (tiip) {
bool ti_is_intf = false;
if (tiip->is_interface())
ti_is_intf = true;
if (is_intf != ti_is_intf)
{ t = _type; break; }
}
t = t->meet_speculative(ti);
}
}
Expand All @@ -1239,60 +1209,13 @@ const Type* PhiNode::Value(PhaseGVN* phase) const {
// The following logic has been moved into TypeOopPtr::filter.
const Type* jt = t->join_speculative(_type);
if (jt->empty()) { // Emptied out???

// Check for evil case of 't' being a class and '_type' expecting an
// interface. This can happen because the bytecodes do not contain
// enough type info to distinguish a Java-level interface variable
// from a Java-level object variable. If we meet 2 classes which
// both implement interface I, but their meet is at 'j/l/O' which
// doesn't implement I, we have no way to tell if the result should
// be 'I' or 'j/l/O'. Thus we'll pick 'j/l/O'. If this then flows
// into a Phi which "knows" it's an Interface type we'll have to
// uplift the type.
if (!t->empty() && ttip && ttip->is_interface()) {
assert(ft == _type, ""); // Uplift to interface
} else if (!t->empty() && ttkp && ttkp->is_interface()) {
assert(ft == _type, ""); // Uplift to interface
} else {
// We also have to handle 'evil cases' of interface- vs. class-arrays
Type::get_arrays_base_elements(jt, _type, NULL, &ttip);
if (!t->empty() && ttip != NULL && ttip->is_interface()) {
assert(ft == _type, ""); // Uplift to array of interface
} else {
// Otherwise it's something stupid like non-overlapping int ranges
// found on dying counted loops.
assert(ft == Type::TOP, ""); // Canonical empty value
}
}
// Otherwise it's something stupid like non-overlapping int ranges
// found on dying counted loops.
assert(ft == Type::TOP, ""); // Canonical empty value
}

else {

// If we have an interface-typed Phi and we narrow to a class type, the join
// should report back the class. However, if we have a J/L/Object
// class-typed Phi and an interface flows in, it's possible that the meet &
// join report an interface back out. This isn't possible but happens
// because the type system doesn't interact well with interfaces.
const TypePtr *jtp = jt->make_ptr();
const TypeInstPtr *jtip = (jtp != NULL) ? jtp->isa_instptr() : NULL;
const TypeInstKlassPtr *jtkp = (jtp != NULL) ? jtp->isa_instklassptr() : NULL;
if (jtip && ttip) {
if (jtip->is_interface() &&
!ttip->is_interface()) {
assert(ft == ttip->cast_to_ptr_type(jtip->ptr()) ||
ft->isa_narrowoop() && ft->make_ptr() == ttip->cast_to_ptr_type(jtip->ptr()), "");
jt = ft;
}
}
if (jtkp && ttkp) {
if (jtkp->is_interface() &&
!jtkp->klass_is_exact() && // Keep exact interface klass (6894807)
ttkp->is_loaded() && !ttkp->is_interface()) {
assert(ft == ttkp->cast_to_ptr_type(jtkp->ptr()) ||
ft->isa_narrowklass() && ft->make_ptr() == ttkp->cast_to_ptr_type(jtkp->ptr()), "");
jt = ft;
}
}
if (jt != ft && jt->base() == ft->base()) {
if (jt->isa_int() &&
jt->is_int()->_lo == ft->is_int()->_lo &&
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/doCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ void Parse::do_call() {

if (receiver_constraint != NULL) {
Node* receiver_node = stack(sp() - nargs);
Node* cls_node = makecon(TypeKlassPtr::make(receiver_constraint));
Node* cls_node = makecon(TypeKlassPtr::make(receiver_constraint, Type::trust_interfaces));
Node* bad_type_ctrl = NULL;
Node* casted_receiver = gen_checkcast(receiver_node, cls_node, &bad_type_ctrl);
if (bad_type_ctrl != NULL) {
Expand Down
14 changes: 7 additions & 7 deletions src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,7 @@ Node* GraphKit::record_profile_for_speculation(Node* n, ciKlass* exact_kls, Prof

// Should the klass from the profile be recorded in the speculative type?
if (current_type->would_improve_type(exact_kls, jvms()->depth())) {
const TypeKlassPtr* tklass = TypeKlassPtr::make(exact_kls);
const TypeKlassPtr* tklass = TypeKlassPtr::make(exact_kls, Type::trust_interfaces);
const TypeOopPtr* xtype = tklass->as_instance_type();
assert(xtype->klass_is_exact(), "Should be exact");
// Any reason to believe n is not null (from this profiling or a previous one)?
Expand Down Expand Up @@ -2636,7 +2636,7 @@ static IfNode* gen_subtype_check_compare(Node* ctrl, Node* in1, Node* in2, BoolT
case T_ADDRESS: cmp = new CmpPNode(in1, in2); break;
default: fatal("unexpected comparison type %s", type2name(bt));
}
gvn.transform(cmp);
cmp = gvn.transform(cmp);
Node* bol = gvn.transform(new BoolNode(cmp, test));
IfNode* iff = new IfNode(ctrl, bol, p, COUNT_UNKNOWN);
gvn.transform(iff);
Expand Down Expand Up @@ -2844,7 +2844,7 @@ Node* GraphKit::type_check_receiver(Node* receiver, ciKlass* klass,
Node* *casted_receiver) {
assert(!klass->is_interface(), "no exact type check on interfaces");

const TypeKlassPtr* tklass = TypeKlassPtr::make(klass);
const TypeKlassPtr* tklass = TypeKlassPtr::make(klass, Type::trust_interfaces);
Node* recv_klass = load_object_klass(receiver);
Node* want_klass = makecon(tklass);
Node* cmp = _gvn.transform(new CmpPNode(recv_klass, want_klass));
Expand Down Expand Up @@ -2873,7 +2873,7 @@ Node* GraphKit::type_check_receiver(Node* receiver, ciKlass* klass,
//------------------------------subtype_check_receiver-------------------------
Node* GraphKit::subtype_check_receiver(Node* receiver, ciKlass* klass,
Node** casted_receiver) {
const TypeKlassPtr* tklass = TypeKlassPtr::make(klass);
const TypeKlassPtr* tklass = TypeKlassPtr::make(klass, Type::trust_interfaces)->try_improve();
Node* want_klass = makecon(tklass);

Node* slow_ctl = gen_subtype_check(receiver, want_klass);
Expand Down Expand Up @@ -2996,7 +2996,7 @@ Node* GraphKit::maybe_cast_profiled_receiver(Node* not_null_obj,
ciKlass* exact_kls = spec_klass == NULL ? profile_has_unique_klass() : spec_klass;
if (exact_kls != NULL) {// no cast failures here
if (require_klass == NULL ||
C->static_subtype_check(require_klass, TypeKlassPtr::make(exact_kls)) == Compile::SSC_always_true) {
C->static_subtype_check(require_klass, TypeKlassPtr::make(exact_kls, Type::trust_interfaces)) == Compile::SSC_always_true) {
// If we narrow the type to match what the type profile sees or
// the speculative type, we can then remove the rest of the
// cast.
Expand Down Expand Up @@ -3182,8 +3182,8 @@ Node* GraphKit::gen_instanceof(Node* obj, Node* superklass, bool safe_for_replac
Node* GraphKit::gen_checkcast(Node *obj, Node* superklass,
Node* *failure_control) {
kill_dead_locals(); // Benefit all the uncommon traps
const TypeKlassPtr *tk = _gvn.type(superklass)->is_klassptr();
const Type *toop = tk->cast_to_exactness(false)->as_instance_type();
const TypeKlassPtr *tk = _gvn.type(superklass)->is_klassptr()->try_improve();
const TypeOopPtr *toop = tk->cast_to_exactness(false)->as_instance_type();

// Fast cutout: Check the case that the cast is vacuously true.
// This detects the common cases where the test will short-circuit
Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/opto/idealGraphPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,9 +500,7 @@ void IdealGraphPrinter::visit_node(Node *n, bool edges, VectorSet* temp_set) {
if (t != NULL && (t->isa_instptr() || t->isa_instklassptr())) {
const TypeInstPtr *toop = t->isa_instptr();
const TypeInstKlassPtr *tkls = t->isa_instklassptr();
if ((toop != NULL && toop->is_interface()) || (tkls != NULL && tkls->is_interface())) {
s2.print(" Interface:");
} else if (toop) {
if (toop) {
s2.print(" Oop:");
} else if (tkls) {
s2.print(" Klass:");
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3659,7 +3659,7 @@ bool LibraryCallKit::inline_Class_cast() {
// Don't use intrinsic when class is not loaded.
return false;
} else {
int static_res = C->static_subtype_check(TypeKlassPtr::make(tm->as_klass()), tp->as_klass_type());
int static_res = C->static_subtype_check(TypeKlassPtr::make(tm->as_klass(), Type::trust_interfaces), tp->as_klass_type());
if (static_res == Compile::SSC_always_true) {
// isInstance() is true - fold the code.
set_result(obj);
Expand Down Expand Up @@ -7161,7 +7161,7 @@ bool LibraryCallKit::inline_digestBase_implCompressMB(Node* digestBase_obj, ciIn
BasicType elem_type, address stubAddr, const char *stubName,
Node* src_start, Node* ofs, Node* limit) {
const TypeKlassPtr* aklass = TypeKlassPtr::make(instklass_digestBase);
const TypeOopPtr* xtype = aklass->as_instance_type()->cast_to_ptr_type(TypePtr::NotNull);
const TypeOopPtr* xtype = aklass->cast_to_exactness(false)->as_instance_type()->cast_to_ptr_type(TypePtr::NotNull);
Node* digest_obj = new CheckCastPPNode(control(), digestBase_obj, xtype);
digest_obj = _gvn.transform(digest_obj);

Expand Down

1 comment on commit 45d1807

@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.