Skip to content
Permalink
Browse files
8253592: [lworld] C2: Additional fixes for handling empty inline types
  • Loading branch information
TobiHartmann committed Sep 24, 2020
1 parent 2989eaa commit 33bdce230cb339442e5e28db233e1b8895fd3555
Showing 10 changed files with 159 additions and 87 deletions.
@@ -780,7 +780,7 @@ void ciTypeFlow::StateVector::do_multianewarray(ciBytecodeStream* str) {
void ciTypeFlow::StateVector::do_new(ciBytecodeStream* str) {
bool will_link;
ciKlass* klass = str->get_klass(will_link);
if (!will_link || str->is_unresolved_klass()) {
if (!will_link || str->is_unresolved_klass() || klass->is_inlinetype()) {
trap(str, klass, str->get_klass_index());
} else {
push_object(klass);
@@ -792,10 +792,9 @@ void ciTypeFlow::StateVector::do_new(ciBytecodeStream* str) {
void ciTypeFlow::StateVector::do_defaultvalue(ciBytecodeStream* str) {
bool will_link;
ciKlass* klass = str->get_klass(will_link);
if (!will_link) {
if (!will_link || !klass->is_inlinetype()) {
trap(str, klass, str->get_klass_index());
} else {
assert(klass->is_inlinetype(), "should be inline type");
push_object(klass);
}
}
@@ -868,7 +868,7 @@ JVMState* Compile::build_start_state(StartNode* start, const TypeFunc* tf) {
for (uint i = 0; i < (uint)arg_size; i++) {
const Type* t = tf->domain_sig()->field_at(i);
Node* parm = NULL;
if (has_scalarized_args() && t->is_inlinetypeptr() && !t->maybe_null()) {
if (has_scalarized_args() && t->is_inlinetypeptr() && !t->maybe_null() && t->inline_klass()->can_be_passed_as_fields()) {
// Inline type arguments are not passed by reference: we get an argument per
// field of the inline type. Build InlineTypeNodes from the inline type arguments.
GraphKit kit(jvms, &gvn);
@@ -120,7 +120,6 @@ void Parse::do_field_access(bool is_get, bool is_field) {

void Parse::do_get_xxx(Node* obj, ciField* field) {
BasicType bt = field->layout_type();

// Does this field have a constant value? If so, just push the value.
if (field->is_constant() &&
// Keep consistent with types found by ciTypeFlow: for an
@@ -138,62 +137,52 @@ void Parse::do_get_xxx(Node* obj, ciField* field) {
}

ciType* field_klass = field->type();
bool is_vol = field->is_volatile();
bool flattened = field->is_flattened();

// Compute address and memory type.
int offset = field->offset_in_bytes();
const TypePtr* adr_type = C->alias_type(field)->adr_type();
Node *adr = basic_plus_adr(obj, obj, offset);

// Build the resultant type of the load
const Type *type;

bool must_assert_null = false;

DecoratorSet decorators = IN_HEAP;
decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED;

bool is_obj = is_reference_type(bt);

if (is_obj) {
if (!field->type()->is_loaded()) {
type = TypeInstPtr::BOTTOM;
must_assert_null = true;
} else if (field->is_static_constant()) {
// This can happen if the constant oop is non-perm.
ciObject* con = field->constant_value().as_object();
// Do not "join" in the previous type; it doesn't add value,
// and may yield a vacuous result if the field is of interface type.
if (con->is_null_object()) {
type = TypePtr::NULL_PTR;
Node* ld = NULL;
if (bt == T_INLINE_TYPE && field_klass->as_inline_klass()->is_empty()) {
// Loading from a field of an empty inline type. Just return the default instance.
ld = InlineTypeNode::make_default(_gvn, field_klass->as_inline_klass());
} else if (field->is_flattened()) {
// Loading from a flattened inline type field.
ld = InlineTypeNode::make_from_flattened(this, field_klass->as_inline_klass(), obj, obj, field->holder(), offset);
} else {
// Build the resultant type of the load
const Type* type;
if (is_reference_type(bt)) {
if (!field_klass->is_loaded()) {
type = TypeInstPtr::BOTTOM;
must_assert_null = true;
} else if (field->is_static_constant()) {
// This can happen if the constant oop is non-perm.
ciObject* con = field->constant_value().as_object();
// Do not "join" in the previous type; it doesn't add value,
// and may yield a vacuous result if the field is of interface type.
if (con->is_null_object()) {
type = TypePtr::NULL_PTR;
} else {
type = TypeOopPtr::make_from_constant(con)->isa_oopptr();
}
assert(type != NULL, "field singleton type must be consistent");
} else {
type = TypeOopPtr::make_from_constant(con)->isa_oopptr();
}
assert(type != NULL, "field singleton type must be consistent");
} else {
type = TypeOopPtr::make_from_klass(field_klass->as_klass());
if (bt == T_INLINE_TYPE && field->is_static()) {
// Check if static inline type field is already initialized
assert(!flattened, "static fields should not be flattened");
ciInstance* mirror = field->holder()->java_mirror();
ciObject* val = mirror->field_value(field).as_object();
if (!val->is_null_object()) {
type = type->join_speculative(TypePtr::NOTNULL);
type = TypeOopPtr::make_from_klass(field_klass->as_klass());
if (bt == T_INLINE_TYPE && field->is_static()) {
// Check if static inline type field is already initialized
ciInstance* mirror = field->holder()->java_mirror();
ciObject* val = mirror->field_value(field).as_object();
if (!val->is_null_object()) {
type = type->join_speculative(TypePtr::NOTNULL);
}
}
}
} else {
type = Type::get_const_basic_type(bt);
}
} else {
type = Type::get_const_basic_type(bt);
}

Node* ld = NULL;
if (flattened) {
// Load flattened inline type
ld = InlineTypeNode::make_from_flattened(this, field_klass->as_inline_klass(), obj, obj, field->holder(), offset);
} else {
Node* adr = basic_plus_adr(obj, obj, offset);
const TypePtr* adr_type = C->alias_type(field)->adr_type();
DecoratorSet decorators = IN_HEAP;
decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED;
decorators |= field->is_volatile() ? MO_SEQ_CST : MO_UNORDERED;
ld = access_load_at(obj, adr, adr_type, type, bt, decorators);
if (bt == T_INLINE_TYPE) {
// Load a non-flattened inline type from memory
@@ -227,7 +216,7 @@ void Parse::do_get_xxx(Node* obj, ciField* field) {
}
if (C->log() != NULL) {
C->log()->elem("assert_null reason='field' klass='%d'",
C->log()->identify(field->type()));
C->log()->identify(field_klass));
}
// If there is going to be a trap, put it at the next bytecode:
set_bci(iter().next_bci());
@@ -238,48 +227,36 @@ void Parse::do_get_xxx(Node* obj, ciField* field) {

void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
bool is_vol = field->is_volatile();

// Compute address and memory type.
int offset = field->offset_in_bytes();
const TypePtr* adr_type = C->alias_type(field)->adr_type();
Node* adr = basic_plus_adr(obj, obj, offset);
BasicType bt = field->layout_type();
// Value to be stored
Node* val = type2size[bt] == 1 ? pop() : pop_pair();

DecoratorSet decorators = IN_HEAP;
decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED;

bool is_obj = is_reference_type(bt);
// Store the value.
const Type* field_type;
if (!field->type()->is_loaded()) {
field_type = TypeInstPtr::BOTTOM;
} else {
if (is_obj) {
field_type = TypeOopPtr::make_from_klass(field->type()->as_klass());
} else {
field_type = Type::BOTTOM;
}
}

if (bt == T_INLINE_TYPE && !val->is_InlineType()) {
// We can see a null constant here
assert(val->bottom_type()->remove_speculative() == TypePtr::NULL_PTR, "Anything other than null?");
push(null());
uncommon_trap(Deoptimization::Reason_null_check, Deoptimization::Action_none);
assert(stopped(), "dead path");
assert(bt != T_INLINE_TYPE || val->is_InlineType() || !gvn().type(val)->maybe_null(), "Null store to inline type field");
if (bt == T_INLINE_TYPE && field->type()->as_inline_klass()->is_empty()) {
// Storing to a field of an empty inline type. Ignore.
return;
}

if (field->is_flattened()) {
// Store flattened inline type to a non-static field
} else if (field->is_flattened()) {
// Storing to a flattened inline type field.
if (!val->is_InlineType()) {
assert(!gvn().type(val)->maybe_null(), "should never be null");
val = InlineTypeNode::make_from_oop(this, val, field->type()->as_inline_klass());
}
val->as_InlineType()->store_flattened(this, obj, obj, field->holder(), offset, decorators);
val->as_InlineType()->store_flattened(this, obj, obj, field->holder(), offset);
} else {
// Store the value.
const Type* field_type;
if (!field->type()->is_loaded()) {
field_type = TypeInstPtr::BOTTOM;
} else {
if (is_reference_type(bt)) {
field_type = TypeOopPtr::make_from_klass(field->type()->as_klass());
} else {
field_type = Type::BOTTOM;
}
}
Node* adr = basic_plus_adr(obj, obj, offset);
const TypePtr* adr_type = C->alias_type(field)->adr_type();
DecoratorSet decorators = IN_HEAP;
decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED;
inc_sp(1);
access_store_at(obj, adr, adr_type, val, field_type, bt, decorators);
dec_sp(1);
@@ -281,6 +281,7 @@ void Parse::do_new() {
bool will_link;
ciInstanceKlass* klass = iter().get_klass(will_link)->as_instance_klass();
assert(will_link, "_new: typeflow responsibility");
assert(!klass->is_inlinetype(), "unexpected inline type");

// Should throw an InstantiationError?
if (klass->is_abstract() || klass->is_interface() ||
@@ -231,6 +231,7 @@ public abstract class InlineTypeTest {
protected static final String CHECKCAST_ARRAY = "(cmp.*precise klass \\[(L|Q)compiler/valhalla/inlinetypes/MyValue.*" + END;
protected static final String CHECKCAST_ARRAYCOPY = "(.*call_leaf_nofp,runtime checkcast_arraycopy.*" + END;
protected static final String JLONG_ARRAYCOPY = "(.*call_leaf_nofp,runtime jlong_disjoint_arraycopy.*" + END;
protected static final String FIELD_ACCESS = "(.*Field: *" + END;

public static String[] concat(String prefix[], String... extra) {
ArrayList<String> list = new ArrayList<String>();
@@ -340,6 +340,7 @@ public int test12() {
}
}

@DontCompile
public void test12_verifier(boolean warmup) {
Asserts.assertEQ(test12(), rI);
}
@@ -361,6 +362,7 @@ public int test13() {
}
}

@DontCompile
public void test13_verifier(boolean warmup) {
Asserts.assertEQ(test13(), rI);
}
@@ -371,6 +373,7 @@ public int test14(MyValue1[] va, int index) {
return va[index].x;
}

@DontCompile
public void test14_verifier(boolean warmup) {
int arraySize = Math.abs(rI) % 10;
MyValue1[] va = new MyValue1[arraySize];
@@ -406,6 +409,7 @@ public int test15() {
}
}

@DontCompile
public void test15_verifier(boolean warmup) {
Asserts.assertEQ(test15(), rI);
}
@@ -426,6 +430,7 @@ public int test16() {
}
}

@DontCompile
public void test16_verifier(boolean warmup) {
Asserts.assertEQ(test16(), rI);
}
@@ -901,4 +901,17 @@ public void test40_verifier(boolean warmup) {
LargeValueWithoutOops res = test40(vt);
Asserts.assertEQ(res, vt);
}

// Test passing/returning an empty inline type together with non-empty
// inline types such that only some inline type arguments are scalarized.
@Test(failOn = ALLOC + LOAD + STORE + TRAP)
public MyValueEmpty test41(MyValue1 vt1, MyValueEmpty vt2, MyValue1 vt3) {
return vt2.copy(vt2);
}

@DontCompile
public void test41_verifier(boolean warmup) {
MyValueEmpty res = test41(MyValue1.default, MyValueEmpty.default, MyValue1.default);
Asserts.assertEQ(res, MyValueEmpty.default);
}
}
@@ -67,6 +67,7 @@ public boolean test1(Class<?> supercls, Class<?> subcls) {
return supercls.isAssignableFrom(subcls);
}

@DontCompile
public void test1_verifier(boolean warmup) {
Asserts.assertTrue(test1(java.util.AbstractList.class, java.util.ArrayList.class), "test1_1 failed");
Asserts.assertTrue(test1(MyValue1.ref.class, MyValue1.ref.class), "test1_2 failed");
@@ -96,6 +97,7 @@ public boolean test2() {
return check1 && check2 && check3 && check4 && check5 && check6 && check7 && check8 && check9 && check10;
}

@DontCompile
public void test2_verifier(boolean warmup) {
Asserts.assertTrue(test2(), "test2 failed");
}
@@ -106,6 +108,7 @@ public Class<?> test3(Class<?> cls) {
return cls.getSuperclass();
}

@DontCompile
public void test3_verifier(boolean warmup) {
Asserts.assertTrue(test3(Object.class) == null, "test3_1 failed");
Asserts.assertTrue(test3(MyValue1.ref.class) == MyAbstract.class, "test3_2 failed");
@@ -125,6 +128,7 @@ public boolean test4() {
return check1 && check2 && check3 && check4;
}

@DontCompile
public void test4_verifier(boolean warmup) {
Asserts.assertTrue(test4(), "test4 failed");
}
@@ -3305,4 +3305,71 @@ public void test119(boolean deopt) {
public void test119_verifier(boolean warmup) {
test119(!warmup);
}

// Test removal of empty inline type field stores
@Test(failOn = ALLOC + ALLOC_G + LOAD + STORE + FIELD_ACCESS + NULL_CHECK_TRAP + TRAP)
public void test120(MyValueEmpty empty) {
fEmpty1 = empty;
fEmpty3 = empty;
// fEmpty2 and fEmpty4 could be null, store can't be removed
}

@DontCompile
public void test120_verifier(boolean warmup) {
test120(MyValueEmpty.default);
Asserts.assertEquals(fEmpty1, MyValueEmpty.default);
Asserts.assertEquals(fEmpty2, MyValueEmpty.default);
}

// Test removal of empty inline type field loads
@Test(failOn = ALLOC + ALLOC_G + LOAD + STORE + FIELD_ACCESS + NULL_CHECK_TRAP + TRAP)
public boolean test121() {
return fEmpty1.equals(fEmpty3);
// fEmpty2 and fEmpty4 could be null, load can't be removed
}

@DontCompile
public void test121_verifier(boolean warmup) {
boolean res = test121();
Asserts.assertTrue(res);
}

// TODO Disabled until JDK-8253416 is fixed
/*
// Verify that empty inline type field loads check for null holder
@Test()
public MyValueEmpty test122(TestLWorld t) {
return t.fEmpty3;
}
@DontCompile
public void test122_verifier(boolean warmup) {
MyValueEmpty res = test122(this);
Asserts.assertEquals(res, MyValueEmpty.default);
try {
test122(null);
throw new RuntimeException("No NPE thrown");
} catch (NullPointerException e) {
// Expected
}
}
// Verify that empty inline type field stores check for null holder
@Test()
public void test123(TestLWorld t) {
t.fEmpty3 = MyValueEmpty.default;
}
@DontCompile
public void test123_verifier(boolean warmup) {
test123(this);
Asserts.assertEquals(fEmpty3, MyValueEmpty.default);
try {
test123(null);
throw new RuntimeException("No NPE thrown");
} catch (NullPointerException e) {
// Expected
}
}
*/
}

0 comments on commit 33bdce2

Please sign in to comment.