Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2709,7 +2709,18 @@ Node* Phase::gen_subtype_check(Node* subklass, Node* superklass, Node** ctrl, No
// Foo[] fa = blah(); Foo x = fa[0]; fa[1] = x;
// Here, the type of 'fa' is often exact, so the store check
// of fa[1]=x will fold up, without testing the nullness of x.
switch (C->static_subtype_check(superk, subk)) {
//
// Do not skip the static sub type check with StressReflectiveCode during
// parsing (i.e. with ExpandSubTypeCheckAtParseTime) because the
// associated CheckCastNodePP could already be folded when the type
// system can prove it's an impossible type. Therefore, we should also
// do the static sub type check here to ensure control is folded as well.
// Otherwise, the graph is left in a broken state.
// At macro expansion, we would have already folded the SubTypeCheckNode
// being expanded here because we always perform the static sub type
// check in SubTypeCheckNode::sub() regardless of whether
// StressReflectiveCode is set or not.
switch (C->static_subtype_check(superk, subk, !ExpandSubTypeCheckAtParseTime)) {
case Compile::SSC_always_false:
{
Node* always_fail = *ctrl;
Expand Down
11 changes: 8 additions & 3 deletions src/hotspot/share/opto/type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6458,14 +6458,19 @@ template <class T1, class T2> bool TypePtr::maybe_java_subtype_of_helper_for_arr
if (other->klass() == ciEnv::current()->Object_klass() && other->_interfaces->empty() && other_exact) {
return true;
}
int dummy;
bool this_top_or_bottom = (this_one->base_element_type(dummy) == Type::TOP || this_one->base_element_type(dummy) == Type::BOTTOM);
if (!this_one->is_loaded() || !other->is_loaded() || this_top_or_bottom) {
if (!this_one->is_loaded() || !other->is_loaded()) {
return true;
}
if (this_one->is_instance_type(other)) {
return other->klass()->equals(ciEnv::current()->Object_klass()) && other->_interfaces->intersection_with(this_one->_interfaces)->eq(other->_interfaces);
Copy link
Contributor

Choose a reason for hiding this comment

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

TypeInterfaces has a contains method that does intersection_with + eq. Could we use it here? i.e. this_one->_interfaces->contains(other->_interfaces)

Copy link
Member Author

Choose a reason for hiding this comment

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

That would definitely be better but I've seen that there are three other uses of intersection_with + eq. We should probably update them all together but not sure if I should squeeze this in here. Should I follow up with an RFE?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed JDK-8329201 - will do a follow up PR.

}

int dummy;
bool this_top_or_bottom = (this_one->base_element_type(dummy) == Type::TOP || this_one->base_element_type(dummy) == Type::BOTTOM);
if (this_top_or_bottom) {
return true;
}

assert(this_one->is_array_type(other), "");

const T1* other_ary = this_one->is_array_type(other);
Expand Down
121 changes: 121 additions & 0 deletions test/hotspot/jtreg/compiler/types/TestSubTypeCheckWithBottomArray.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test id=Xcomp
* @bug 8328702
* @summary Check that SubTypeCheckNode is properly folded when having an array with bottom type elements checked
* against an interface.
*
* @run main/othervm -Xcomp -XX:CompileCommand=compileonly,compiler.types.TestSubTypeCheckWithBottomArray::test*
* -XX:CompileCommand=inline,compiler.types.TestSubTypeCheckWithBottomArray::check*
* compiler.types.TestSubTypeCheckWithBottomArray
*/

/*
* @test id=Xbatch
* @bug 8328702
* @summary Check that SubTypeCheckNode is properly folded when having an array with bottom type elements checked
* against an interface.
*
* @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.types.TestSubTypeCheckWithBottomArray::test*
* -XX:CompileCommand=inline,compiler.types.TestSubTypeCheckWithBottomArray::check*
* compiler.types.TestSubTypeCheckWithBottomArray
*/

/*
* @test id=stress
* @bug 8328702
* @summary Check that PartialSubtypeCheckNode is properly folded when having an array with bottom type elements checked
* either against an interface or an unrelated non-sub-class.
*
* @run main/othervm -Xcomp -XX:CompileCommand=compileonly,compiler.types.TestSubTypeCheckWithBottomArray::test*
* -XX:+UnlockDiagnosticVMOptions -XX:+ExpandSubTypeCheckAtParseTime
* -XX:+IgnoreUnrecognizedVMOptions -XX:+StressReflectiveCode
* -XX:CompileCommand=inline,compiler.types.TestSubTypeCheckWithBottomArray::check*
* compiler.types.TestSubTypeCheckWithBottomArray
* @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.types.TestSubTypeCheckWithBottomArray::test*
* -XX:+UnlockDiagnosticVMOptions -XX:+ExpandSubTypeCheckAtParseTime
* -XX:+IgnoreUnrecognizedVMOptions -XX:+StressReflectiveCode
* -XX:CompileCommand=inline,compiler.types.TestSubTypeCheckWithBottomArray::check*
* compiler.types.TestSubTypeCheckWithBottomArray
*/

package compiler.types;

public class TestSubTypeCheckWithBottomArray {
static byte[] bArr = new byte[10];
static Object[] oArr = new Object[10];
static boolean flag;

public static void main(String[] args) {
A a = new A();
B b = new B();
Y y = new Y();
Z z = new Z();
for (int i = 0; i < 10000; i++) {
// With -Xcomp: Immediatly crashes because of no profiling -> don't know anything.
checkInterface(a); // Make sure that checkInterface() sometimes passes instanceof.
checkInterface(b); // Use two sub classes such that checkcast is required.
testInterface();

checkClass(y); // Make sure that checkClass() sometimes passes instanceof.
checkClass(z); // Use two sub classes such that checkcast is required.
testClass();
flag = !flag;
}
}

static void testInterface() {
checkInterface(flag ? bArr : oArr); // Inlined, never passes instanceof
}

static void checkInterface(Object o) {
if (o instanceof I i) {
// Use of i: Needs CheckCastPP which is replaced by top because [bottom <: I cannot be true.
// But: SubTypeCheckNode is not folded away -> broken graph (data dies, control not)
i.getClass();
}
}

static void testClass() {
checkClass(flag ? bArr : oArr); // Inlined, never passes instanceof
}

static void checkClass(Object o) {
if (o instanceof X x) {
// Use of i: Needs CheckCastPP which is replaced by top because [bottom <: I cannot be true.
// But: SubTypeCheckNode is not folded away -> broken graph (data dies, control not)
x.getClass();
}
}

}

interface I {}
class A implements I {}
class B implements I {}

class X {}
class Y extends X {}
class Z extends X {}