Skip to content

Commit b5395e8

Browse files
TobiHartmannTobiHartmann
committed
8353202: [lworld] TestArrays.java crashes with -XX:+UseNullableValueFlattening
1 parent f48c0f4 commit b5395e8

File tree

6 files changed

+71
-11
lines changed

6 files changed

+71
-11
lines changed

‎src/hotspot/share/c1/c1_LIRGenerator.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,7 +1503,7 @@ LIR_Opr LIRGenerator::load_constant(Constant* x) {
15031503

15041504
LIR_Opr LIRGenerator::load_constant(LIR_Const* c) {
15051505
BasicType t = c->type();
1506-
for (int i = 0; i < _constants.length(); i++) {
1506+
for (int i = 0; i < _constants.length() && !in_conditional_code(); i++) {
15071507
LIR_Const* other = _constants.at(i);
15081508
if (t == other->type()) {
15091509
switch (t) {
@@ -1970,8 +1970,7 @@ void LIRGenerator::do_StoreIndexed(StoreIndexed* x) {
19701970
decorators |= C1_MASK_BOOLEAN;
19711971
}
19721972

1973-
access_store_at(decorators, x->elt_type(), array, index.result(), value.result(),
1974-
nullptr, null_check_info);
1973+
access_store_at(decorators, x->elt_type(), array, index.result(), value.result(), nullptr, null_check_info);
19751974
if (slow_path != nullptr) {
19761975
__ branch_destination(slow_path->continuation());
19771976
set_in_conditional_code(false);

‎src/hotspot/share/opto/inlinetypenode.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ Node* InlineTypeNode::convert_to_payload(GraphKit* kit, BasicType bt, Node* payl
671671
if (!null_free) {
672672
// Set the null marker
673673
value = get_is_init();
674-
payload = set_payload_value(gvn, payload, bt, value, T_BYTE, null_marker_offset);
674+
payload = set_payload_value(gvn, payload, bt, value, T_BOOLEAN, null_marker_offset);
675675
}
676676
// Iterate over the fields and add their values to the payload
677677
for (uint i = 0; i < field_count(); ++i) {
@@ -714,7 +714,7 @@ void InlineTypeNode::store_flat(GraphKit* kit, Node* base, Node* ptr, Node* idx,
714714

715715
if (atomic) {
716716
#ifdef ASSERT
717-
bool is_naturally_atomic = vk->is_empty() || (null_free && vk->nof_declared_nonstatic_fields() == 1);
717+
bool is_naturally_atomic = null_free && vk->nof_declared_nonstatic_fields() <= 1;
718718
assert(!is_naturally_atomic, "No atomic access required");
719719
#endif
720720
// Convert to a payload value <= 64-bit and write atomically.
@@ -1293,7 +1293,7 @@ InlineTypeNode* InlineTypeNode::make_from_flat_impl(GraphKit* kit, ciInlineKlass
12931293
if (atomic) {
12941294
// Read atomically and convert from payload
12951295
#ifdef ASSERT
1296-
bool is_naturally_atomic = vk->is_empty() || (null_free && vk->nof_declared_nonstatic_fields() == 1);
1296+
bool is_naturally_atomic = null_free && vk->nof_declared_nonstatic_fields() <= 1;
12971297
assert(!is_naturally_atomic, "No atomic access required");
12981298
#endif
12991299
BasicType bt = vk->atomic_size_to_basic_type(null_free);
@@ -1361,7 +1361,7 @@ InlineTypeNode* InlineTypeNode::make_from_flat_impl(GraphKit* kit, ciInlineKlass
13611361
}
13621362
// Set the null marker if not known to be null-free
13631363
if (!null_free) {
1364-
load = set_payload_value(&kit->gvn(), load, bt, kit->intcon(1), T_BYTE, null_marker_offset);
1364+
load = set_payload_value(&kit->gvn(), load, bt, kit->intcon(1), T_BOOLEAN, null_marker_offset);
13651365
}
13661366
payload_null_free->init_req(1, load);
13671367
}

‎src/hotspot/share/opto/parse2.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ void Parse::array_load(BasicType bt) {
122122
// TODO 8350865 Impossible type
123123
is_not_null_free = false;
124124
}
125-
bool is_naturally_atomic = vk->is_empty() || (is_null_free && vk->nof_declared_nonstatic_fields() == 1);
125+
bool is_naturally_atomic = is_null_free && vk->nof_declared_nonstatic_fields() <= 1;
126126
bool may_need_atomicity = !is_naturally_atomic && ((!is_not_null_free && vk->has_atomic_layout()) || (!is_null_free && vk->has_nullable_atomic_layout()));
127127

128128
adr = flat_array_element_address(array, array_index, vk, is_null_free, is_not_null_free, may_need_atomicity);
@@ -279,7 +279,7 @@ void Parse::array_store(BasicType bt) {
279279
// TODO 8350865 Impossible type
280280
is_not_null_free = false;
281281
}
282-
bool is_naturally_atomic = vk->is_empty() || (is_null_free && vk->nof_declared_nonstatic_fields() == 1);
282+
bool is_naturally_atomic = is_null_free && vk->nof_declared_nonstatic_fields() <= 1;
283283
bool may_need_atomicity = !is_naturally_atomic && ((!is_not_null_free && vk->has_atomic_layout()) || (!is_null_free && vk->has_nullable_atomic_layout()));
284284

285285
// Re-execute flat array store if buffering triggers deoptimization

‎src/hotspot/share/opto/parse3.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ void Parse::do_get_xxx(Node* obj, ciField* field) {
151151
} else if (field->is_flat()) {
152152
// Loading from a flat inline type field.
153153
ciInlineKlass* vk = field->type()->as_inline_klass();
154-
bool is_naturally_atomic = vk->is_empty() || (field->is_null_free() && vk->nof_declared_nonstatic_fields() == 1);
154+
bool is_naturally_atomic = field->is_null_free() && vk->nof_declared_nonstatic_fields() <= 1;
155155
bool needs_atomic_access = (!field->is_null_free() || field->is_volatile()) && !is_naturally_atomic;
156156
ld = InlineTypeNode::make_from_flat(this, field_klass->as_inline_klass(), obj, obj, nullptr, field->holder(), offset, needs_atomic_access, field->null_marker_offset());
157157
} else {
@@ -270,7 +270,7 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
270270
val = InlineTypeNode::make_null(gvn(), vk);
271271
}
272272
inc_sp(1);
273-
bool is_naturally_atomic = vk->is_empty() || (field->is_null_free() && vk->nof_declared_nonstatic_fields() == 1);
273+
bool is_naturally_atomic = field->is_null_free() && vk->nof_declared_nonstatic_fields() <= 1;
274274
bool needs_atomic_access = (!field->is_null_free() || field->is_volatile()) && !is_naturally_atomic;
275275
val->as_InlineType()->store_flat(this, obj, obj, nullptr, field->holder(), offset, needs_atomic_access, field->null_marker_offset(), IN_HEAP | MO_UNORDERED);
276276
dec_sp(1);

‎test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestArrayNullMarkers.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ public static TwoBytes[] testNullableAtomicArrayIntrinsic(int size, int idx, Two
383383
return nullableAtomicArray;
384384
}
385385

386+
@LooselyConsistentValue
387+
static value class MyValueEmpty {
388+
static final MyValueEmpty DEFAULT = new MyValueEmpty();
389+
}
390+
386391
@LooselyConsistentValue
387392
static value class ValueHolder1 {
388393
TwoBytes val;
@@ -779,10 +784,16 @@ public static void main(String[] args) {
779784
ByteAndOop[] nullableArray5 = new ByteAndOop[3];
780785
ByteAndOop[] nullableAtomicArray5 = (ByteAndOop[])ValueClass.newNullableAtomicArray(ByteAndOop.class, 3);
781786

787+
IntAndArrayOop[] nullFreeArray6 = (IntAndArrayOop[])ValueClass.newNullRestrictedNonAtomicArray(IntAndArrayOop.class, 3, IntAndArrayOop.DEFAULT);
782788
IntAndArrayOop[] nullFreeAtomicArray6 = (IntAndArrayOop[])ValueClass.newNullRestrictedAtomicArray(IntAndArrayOop.class, 3, IntAndArrayOop.DEFAULT);
783789
IntAndArrayOop[] nullableArray6 = new IntAndArrayOop[3];
784790
IntAndArrayOop[] nullableAtomicArray6 = (IntAndArrayOop[])ValueClass.newNullableAtomicArray(IntAndArrayOop.class, 3);
785791

792+
MyValueEmpty[] nullFreeArrayEmpty = (MyValueEmpty[])ValueClass.newNullRestrictedNonAtomicArray(MyValueEmpty.class, 3, MyValueEmpty.DEFAULT);
793+
MyValueEmpty[] nullFreeAtomicArrayEmpty = (MyValueEmpty[])ValueClass.newNullRestrictedAtomicArray(MyValueEmpty.class, 3, MyValueEmpty.DEFAULT);
794+
MyValueEmpty[] nullableArrayEmpty = new MyValueEmpty[3];
795+
MyValueEmpty[] nullableAtomicArrayEmpty = (MyValueEmpty[])ValueClass.newNullableAtomicArray(MyValueEmpty.class, 3);
796+
786797
// Write canary values to detect out of bound writes
787798
nullFreeArray0[0] = CANARY0;
788799
nullFreeArray0[2] = CANARY0;
@@ -963,22 +974,50 @@ public static void main(String[] args) {
963974
testWrite5(nullableAtomicArray5, 1, val5);
964975

965976
IntAndArrayOop val6 = new IntAndArrayOop(i, new MyClass[1]);
977+
nullFreeArray6[1] = val6;
978+
nullFreeArray6[2] = nullFreeArray6[1];
966979
nullFreeAtomicArray6[1] = val6;
967980
nullFreeAtomicArray6[2] = nullFreeAtomicArray6[1];
968981
nullableArray6[1] = val6;
969982
nullableArray6[2] = nullableArray6[1];
970983
nullableAtomicArray6[1] = val6;
971984
nullableAtomicArray6[2] = nullableAtomicArray6[1];
985+
Asserts.assertEQ(nullFreeArray6[0], new IntAndArrayOop(0, null));
972986
Asserts.assertEQ(nullFreeAtomicArray6[0], new IntAndArrayOop(0, null));
973987
Asserts.assertEQ(nullableArray6[0], null);
974988
Asserts.assertEQ(nullableAtomicArray6[0], null);
989+
Asserts.assertEQ(nullFreeArray6[1], val6);
975990
Asserts.assertEQ(nullFreeAtomicArray6[1], val6);
976991
Asserts.assertEQ(nullableArray6[1], val6);
977992
Asserts.assertEQ(nullableAtomicArray6[1], val6);
993+
Asserts.assertEQ(nullFreeArray6[2], val6);
978994
Asserts.assertEQ(nullFreeAtomicArray6[2], val6);
979995
Asserts.assertEQ(nullableArray6[2], val6);
980996
Asserts.assertEQ(nullableAtomicArray6[2], val6);
981997

998+
// Test empty arrays
999+
MyValueEmpty valEmpty = new MyValueEmpty();
1000+
nullFreeArrayEmpty[1] = valEmpty;
1001+
nullFreeArrayEmpty[2] = nullFreeArrayEmpty[1];
1002+
nullFreeAtomicArrayEmpty[1] = valEmpty;
1003+
nullFreeAtomicArrayEmpty[2] = nullFreeAtomicArrayEmpty[1];
1004+
nullableArrayEmpty[1] = valEmpty;
1005+
nullableArrayEmpty[2] = nullableArrayEmpty[1];
1006+
nullableAtomicArrayEmpty[1] = valEmpty;
1007+
nullableAtomicArrayEmpty[2] = nullableAtomicArrayEmpty[1];
1008+
Asserts.assertEQ(nullFreeArrayEmpty[0], new MyValueEmpty());
1009+
Asserts.assertEQ(nullFreeAtomicArrayEmpty[0], new MyValueEmpty());
1010+
Asserts.assertEQ(nullableArrayEmpty[0], null);
1011+
Asserts.assertEQ(nullableAtomicArrayEmpty[0], null);
1012+
Asserts.assertEQ(nullFreeArrayEmpty[1], valEmpty);
1013+
Asserts.assertEQ(nullFreeAtomicArrayEmpty[1], valEmpty);
1014+
Asserts.assertEQ(nullableArrayEmpty[1], valEmpty);
1015+
Asserts.assertEQ(nullableAtomicArrayEmpty[1], valEmpty);
1016+
Asserts.assertEQ(nullFreeArrayEmpty[2], valEmpty);
1017+
Asserts.assertEQ(nullFreeAtomicArrayEmpty[2], valEmpty);
1018+
Asserts.assertEQ(nullableArrayEmpty[2], valEmpty);
1019+
Asserts.assertEQ(nullableAtomicArrayEmpty[2], valEmpty);
1020+
9821021
if (i > (LIMIT - 50)) {
9831022
// After warmup, produce some garbage to trigger GC
9841023
produceGarbage();

‎test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestFieldNullMarkers.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,15 @@ public MyValue17(MyClass obj, byte b1, byte b2) {
369369
volatile MyValue17 field20 = new MyValue17(null, (byte)0, (byte)0);
370370
MyValue17 field21;
371371

372+
@Strict
373+
@NullRestricted
374+
MyValueEmpty emptyField1 = new MyValueEmpty();
375+
@Strict
376+
@NullRestricted
377+
volatile MyValueEmpty emptyField2 = new MyValueEmpty();
378+
MyValueEmpty emptyField3;
379+
volatile MyValueEmpty emptyField4;
380+
372381
static final MyValue1 VAL1 = new MyValue1((byte)42, new MyValue2((byte)43), null);
373382
static final MyValue4 VAL4 = new MyValue4(new MyValue3((byte)42), null);
374383
static final MyValue5 VAL5 = new MyValue5((byte)42, new MyValue5_1((byte)43, new MyValue5_2((byte)44, new MyValue5_3((byte)45))));
@@ -1095,6 +1104,19 @@ public static void main(String[] args) {
10951104
Asserts.assertEQ(t.field21.b1, (byte)i);
10961105
Asserts.assertEQ(t.field21.b2, (byte)i);
10971106

1107+
Asserts.assertEQ(t.emptyField1, new MyValueEmpty());
1108+
Asserts.assertEQ(t.emptyField2, new MyValueEmpty());
1109+
1110+
// Test empty fields
1111+
t.emptyField3 = new MyValueEmpty();
1112+
t.emptyField4 = new MyValueEmpty();
1113+
Asserts.assertEQ(t.emptyField3, new MyValueEmpty());
1114+
Asserts.assertEQ(t.emptyField4, new MyValueEmpty());
1115+
t.emptyField3 = null;
1116+
t.emptyField4 = null;
1117+
Asserts.assertEQ(t.emptyField3, null);
1118+
Asserts.assertEQ(t.emptyField4, null);
1119+
10981120
t.testLoadingFromConstantHolder(i);
10991121

11001122
// Verify that no out of bounds accesses happen

0 commit comments

Comments
 (0)