diff --git a/Test.java b/Test.java index 2ae261dca16..7d595226255 100644 --- a/Test.java +++ b/Test.java @@ -81,8 +81,56 @@ public String toString() { } } + @ImplicitlyConstructible + @LooselyConsistentValue + static value class MyValue5_3 { + byte x; + + public MyValue5_3(byte x) { + this.x = x; + } + } + + @ImplicitlyConstructible + @LooselyConsistentValue + static value class MyValue5_2 { + byte x; + MyValue5_3 val; + + public MyValue5_2(byte x, MyValue5_3 val) { + this.x = x; + this.val = val; + } + } + + @ImplicitlyConstructible + @LooselyConsistentValue + static value class MyValue5_1 { + byte x; + MyValue5_2 val; + + public MyValue5_1(byte x, MyValue5_2 val) { + this.x = x; + this.val = val; + } + } + + // Value class with deep nesting of nullable flat fields + @ImplicitlyConstructible + @LooselyConsistentValue + static value class MyValue5 { + byte x; + MyValue5_1 val; + + public MyValue5(byte x, MyValue5_1 val) { + this.x = x; + this.val = val; + } + } + MyValue1 field1; MyValue4 field2; + MyValue5 field3; // Test that the calling convention is keeping track of the null marker public MyValue1 testHelper1(MyValue1 val) { @@ -104,16 +152,16 @@ public void testDeopt1(byte x, MyValue1 neverNull, MyValue1 alwaysNull, boolean if (val1.x != x) throw new RuntimeException("FAIL"); if (val1.val1 != val2) throw new RuntimeException("FAIL"); if (val1.val2 != val2) throw new RuntimeException("FAIL"); - if (neverNull.x != x) throw new RuntimeException("FAIL"); - if (neverNull.val1 != val2) throw new RuntimeException("FAIL"); + if (neverNull.x != x) throw new RuntimeException("FAIL"); + if (neverNull.val1 != val2) throw new RuntimeException("FAIL"); if (neverNull.val2 != val2) throw new RuntimeException("FAIL"); - if (alwaysNull.x != x) throw new RuntimeException("FAIL"); - if (alwaysNull.val1 != null) throw new RuntimeException("FAIL"); + if (alwaysNull.x != x) throw new RuntimeException("FAIL"); + if (alwaysNull.val1 != null) throw new RuntimeException("FAIL"); if (alwaysNull.val2 != null) throw new RuntimeException("FAIL"); } } - public void testOSR1() { + public void testOSR() { // Trigger OSR for (int i = 0; i < 100_000; ++i) { field1 = null; @@ -157,9 +205,62 @@ public void testDeopt2(byte x, MyValue4 neverNull, MyValue4 alwaysNull, boolean } } + // Test that the calling convention is keeping track of the null marker + public MyValue5 testHelper3(MyValue5 val) { + return val; + } + + public void testSet3(MyValue5 val) { + field3 = testHelper3(val); + } + + public MyValue5 testGet3() { + return field3; + } + + public void testDeopt3(byte x, MyValue5 val6, MyValue5 val7, MyValue5 val8, MyValue5 val9, boolean deopt) { + MyValue5 val1 = new MyValue5(x, new MyValue5_1(x, new MyValue5_2(x, new MyValue5_3(x)))); + MyValue5 val2 = new MyValue5(x, new MyValue5_1(x, new MyValue5_2(x, null))); + MyValue5 val3 = new MyValue5(x, new MyValue5_1(x, null)); + MyValue5 val4 = new MyValue5(x, null); + MyValue5 val5 = null; + if (deopt) { + if (val1.x != x) throw new RuntimeException("FAIL"); + if (val1.val.x != x) throw new RuntimeException("FAIL"); + if (val1.val.val.x != x) throw new RuntimeException("FAIL"); + if (val1.val.val.val.x != x) throw new RuntimeException("FAIL"); + if (val2.x != x) throw new RuntimeException("FAIL"); + if (val2.val.x != x) throw new RuntimeException("FAIL"); + if (val2.val.val.x != x) throw new RuntimeException("FAIL"); + if (val2.val.val.val != null) throw new RuntimeException("FAIL"); + if (val3.x != x) throw new RuntimeException("FAIL"); + if (val3.val.x != x) throw new RuntimeException("FAIL"); + if (val3.val.val != null) throw new RuntimeException("FAIL"); + if (val4.x != x) throw new RuntimeException("FAIL"); + if (val4.val != null) throw new RuntimeException("FAIL"); + if (val5 != null) throw new RuntimeException("FAIL"); + + if (val6.x != x) throw new RuntimeException("FAIL"); + if (val6.val.x != x) throw new RuntimeException("FAIL"); + if (val6.val.val.x != x) throw new RuntimeException("FAIL"); + if (val6.val.val.val.x != x) throw new RuntimeException("FAIL"); + if (val7.x != x) throw new RuntimeException("FAIL"); + if (val7.val.x != x) throw new RuntimeException("FAIL"); + if (val7.val.val.x != x) throw new RuntimeException("FAIL"); + if (val7.val.val.val != null) throw new RuntimeException("FAIL"); + if (val8.x != x) throw new RuntimeException("FAIL"); + if (val8.val.x != x) throw new RuntimeException("FAIL"); + if (val8.val.val != null) throw new RuntimeException("FAIL"); + if (val9.x != x) throw new RuntimeException("FAIL"); + if (val9.val != null) throw new RuntimeException("FAIL"); + } + } + +// TODO add test with empty inline types + public static void main(String[] args) { Test t = new Test(); - t.testOSR1(); + t.testOSR(); for (int i = 0; i < 100_000; ++i) { t.field1 = null; if (t.testGet1() != null) throw new RuntimeException("FAIL1"); @@ -205,10 +306,67 @@ public static void main(String[] args) { if (t.field2.val2 != val3) throw new RuntimeException("FAIL3"); t.testDeopt2((byte)i, null, null, false); + + t.field3 = null; + if (t.testGet3() != null) throw new RuntimeException("FAIL1"); + + boolean useNull_1 = (i % 4) == 0; + boolean useNull_2 = (i % 4) == 1; + boolean useNull_3 = (i % 4) == 2; + MyValue5_3 val5_3 = useNull_3 ? null : new MyValue5_3((byte)i); + MyValue5_2 val5_2 = useNull_2 ? null : new MyValue5_2((byte)i, val5_3); + MyValue5_1 val5_1 = useNull_1 ? null : new MyValue5_1((byte)i, val5_2); + MyValue5 val5 = new MyValue5((byte)i, val5_1); + t.field3 = val5; + if (t.field3.x != val5.x) throw new RuntimeException("FAIL3"); + if (useNull_1) { + if (t.field3.val != null) throw new RuntimeException("FAIL3"); + } else { + if (t.field3.val.x != val5_1.x) throw new RuntimeException("FAIL3"); + if (useNull_2) { + if (t.field3.val.val != null) throw new RuntimeException("FAIL3"); + } else { + if (t.field3.val.val.x != val5_2.x) throw new RuntimeException("FAIL3"); + if (useNull_3) { + if (t.field3.val.val.val != null) throw new RuntimeException("FAIL3"); + } else { + if (t.field3.val.val.val.x != val5_3.x) throw new RuntimeException("FAIL3"); + } + } + } + + t.testSet3(null); + if (t.field3 != null) throw new RuntimeException("FAIL3"); + + t.testSet3(val5); + if (t.field3.x != val5.x) throw new RuntimeException("FAIL3"); + if (useNull_1) { + if (t.field3.val != null) throw new RuntimeException("FAIL3"); + } else { + if (t.field3.val.x != val5_1.x) throw new RuntimeException("FAIL3"); + if (useNull_2) { + if (t.field3.val.val != null) throw new RuntimeException("FAIL3"); + } else { + if (t.field3.val.val.x != val5_2.x) throw new RuntimeException("FAIL3"); + if (useNull_3) { + if (t.field3.val.val.val != null) throw new RuntimeException("FAIL3"); + } else { + if (t.field3.val.val.val.x != val5_3.x) throw new RuntimeException("FAIL3"); + } + } + } + t.testDeopt3((byte)i, null, null, null, null, false); } // Trigger deoptimization to check that re-materialization takes the null marker into account - t.testDeopt1((byte)42, new MyValue1((byte)42, new MyValue2((byte)42), new MyValue2((byte)42)), new MyValue1((byte)42, null, null), true); - t.testDeopt2((byte)42, new MyValue4(new MyValue3((byte)42), new MyValue3((byte)42)), new MyValue4(null, null), true); + byte x = (byte)42; + t.testDeopt1(x, new MyValue1(x, new MyValue2(x), new MyValue2(x)), new MyValue1(x, null, null), true); + t.testDeopt2(x, new MyValue4(new MyValue3(x), new MyValue3(x)), new MyValue4(null, null), true); + + MyValue5 val1 = new MyValue5(x, new MyValue5_1(x, new MyValue5_2(x, new MyValue5_3(x)))); + MyValue5 val2 = new MyValue5(x, new MyValue5_1(x, new MyValue5_2(x, null))); + MyValue5 val3 = new MyValue5(x, new MyValue5_1(x, null)); + MyValue5 val4 = new MyValue5(x, null); + t.testDeopt3(x, val1, val2, val3, val4, true); } } diff --git a/src/hotspot/share/oops/inlineKlass.cpp b/src/hotspot/share/oops/inlineKlass.cpp index 011203d77c2..1673d8f0bf0 100644 --- a/src/hotspot/share/oops/inlineKlass.cpp +++ b/src/hotspot/share/oops/inlineKlass.cpp @@ -294,49 +294,51 @@ Klass* InlineKlass::value_array_klass_or_null() { // // Value classes could also have fields in abstract super value classes. // Use a HierarchicalFieldStream to get them as well. -int InlineKlass::collect_fields(GrowableArray* sig, int base_off, int null_marker_offset) { +int InlineKlass::collect_fields(GrowableArray* sig, float& max_offset, int base_off, int null_marker_offset) { int count = 0; SigEntry::add_entry(sig, T_METADATA, name(), base_off); - int max_offset = 0; + max_offset = base_off; for (HierarchicalFieldStream fs(this); !fs.done(); fs.next()) { if (fs.access_flags().is_static()) continue; int offset = base_off + fs.offset() - (base_off > 0 ? first_field_offset() : 0); // TODO 8284443 Use different heuristic to decide what should be scalarized in the calling convention if (fs.is_flat()) { // Resolve klass of flat field and recursively collect fields - int null_marker_offset = -1; + int field_null_marker_offset = -1; if (!fs.is_null_free_inline_type()) { // TODO can we use null_marker_offset() instead? InlineLayoutInfo* li = inline_layout_info_adr(fs.index()); - null_marker_offset = li->null_marker_offset(); + field_null_marker_offset = base_off + li->null_marker_offset() - (base_off > 0 ? first_field_offset() : 0); } Klass* vk = get_inline_type_field_klass(fs.index()); - count += InlineKlass::cast(vk)->collect_fields(sig, offset, null_marker_offset); + count += InlineKlass::cast(vk)->collect_fields(sig, max_offset, offset, field_null_marker_offset); } else { BasicType bt = Signature::basic_type(fs.signature()); SigEntry::add_entry(sig, bt, fs.signature(), offset); count += type2size[bt]; } - max_offset = MAX2(max_offset, offset); + max_offset = MAX2(max_offset, (float)offset); } int offset = base_off + size_helper()*HeapWordSize - (base_off > 0 ? first_field_offset() : 0); // Null markers are no real fields, add them manually at the end (C2 relies on this) of the flat fields if (null_marker_offset != -1) { - tty->print_cr("MARKER %d %d %d", null_marker_offset, offset, max_offset + 1); - SigEntry::add_entry(sig, T_BOOLEAN, name(), null_marker_offset, max_offset + 1); + max_offset += 0.1f; // We add the markers "in-between" because they are no real fields + tty->print_cr("MARKER %d %d %f", null_marker_offset, offset, max_offset); + SigEntry::add_entry(sig, T_BOOLEAN, name(), null_marker_offset, max_offset); count++; } + // TODO I think this is incorrect SigEntry::add_entry(sig, T_VOID, name(), offset); if (base_off == 0) { sig->sort(SigEntry::compare); } - /* + tty->print_cr("##\n"); print(); for (int i = 0; i < sig->length(); ++i) { - tty->print_cr("%s %d %d", type2name(sig->at(i)._bt), sig->at(i)._offset, sig->at(i)._sort_offset); + tty->print_cr("%s %d %f %s", type2name(sig->at(i)._bt), sig->at(i)._offset, sig->at(i)._sort_offset, (sig->at(i)._offset != sig->at(i)._sort_offset) ? " MARKER" : ""); } - */ + assert(sig->at(0)._bt == T_METADATA && sig->at(sig->length()-1)._bt == T_VOID, "broken structure"); return count; } @@ -348,7 +350,8 @@ void InlineKlass::initialize_calling_convention(TRAPS) { if (InlineTypeReturnedAsFields || InlineTypePassFieldsAsArgs) { ResourceMark rm; GrowableArray sig_vk; - int nb_fields = collect_fields(&sig_vk); + float max_offset = 0; + int nb_fields = collect_fields(&sig_vk, max_offset); Array* extended_sig = MetadataFactory::new_array(class_loader_data(), sig_vk.length(), CHECK); *((Array**)adr_extended_sig()) = extended_sig; for (int i = 0; i < sig_vk.length(); i++) { diff --git a/src/hotspot/share/oops/inlineKlass.hpp b/src/hotspot/share/oops/inlineKlass.hpp index 615e60cf683..5db185d0a6d 100644 --- a/src/hotspot/share/oops/inlineKlass.hpp +++ b/src/hotspot/share/oops/inlineKlass.hpp @@ -178,7 +178,7 @@ class InlineKlass: public InstanceKlass { virtual void metaspace_pointers_do(MetaspaceClosure* it); private: - int collect_fields(GrowableArray* sig, int base_off = 0, int null_marker_offset = -1); + int collect_fields(GrowableArray* sig, float& max_offset, int base_off = 0, int null_marker_offset = -1); void cleanup_blobs(); diff --git a/src/hotspot/share/opto/inlinetypenode.cpp b/src/hotspot/share/opto/inlinetypenode.cpp index b1602ffd047..800d3b79f77 100644 --- a/src/hotspot/share/opto/inlinetypenode.cpp +++ b/src/hotspot/share/opto/inlinetypenode.cpp @@ -192,6 +192,32 @@ Node* InlineTypeNode::field_value(uint index) const { return in(Values + index); } + +// TODO implement with a worklist +/* +static uint helper2(InlineTypeNode* vt, Unique_Node_List& worklist, Node_List& null_markers) { + uint cnt = 0; + for (uint i = 0; i < vt->field_count(); ++i) { + Node* value = vt->field_value(i); + if (vt->field_is_flat(i)) { + cnt += helper(value->as_InlineType(), worklist, null_markers, sfpt); + if (!vt->field_is_null_free(i)) { + vt->field_null_marker_offset(i) + null_markers.push(value->as_InlineType()->get_is_init()); + } + } else { + if (value->is_InlineType()) { + // Add inline type field to the worklist to process later + worklist.push(value); + } + sfpt->add_req(value); + cnt++; + } + } + return cnt; +} +*/ + // Get the value of the field at the given offset. // If 'recursive' is true, flat inline type fields will be resolved recursively. Node* InlineTypeNode::field_value_by_offset(int offset, bool recursive) const { @@ -487,7 +513,7 @@ void InlineTypeNode::load(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass* } else if (field_is_flat(i)) { // Recursively load the flat inline type field assert(field_is_null_free(i) == (field_null_marker_offset(i) == -1), "inconsistency"); - value = make_from_flat_impl(kit, ft->as_inline_klass(), base, ptr, holder, offset, field_null_marker_offset(i), decorators, visited); + value = make_from_flat_impl(kit, ft->as_inline_klass(), base, ptr, holder, offset, holder_offset + field_null_marker_offset(i), decorators, visited); } else { const TypeOopPtr* oop_ptr = kit->gvn().type(base)->isa_oopptr(); bool is_array = (oop_ptr->isa_aryptr() != nullptr); @@ -540,7 +566,7 @@ void InlineTypeNode::store_flat(GraphKit* kit, Node* base, Node* ptr, ciInstance holder = inline_klass(); } if (null_marker_offset != -1) { - // Nullable flat field, read the null marker + // Nullable flat field, store the null marker Node* adr = kit->basic_plus_adr(base, null_marker_offset); const TypePtr* adr_type = kit->gvn().type(adr)->isa_ptr(); int alias_idx = kit->C->get_alias_index(adr_type); @@ -560,7 +586,7 @@ void InlineTypeNode::store(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass if (field_is_flat(i)) { // Recursively store the flat inline type field assert(field_is_null_free(i) == (field_null_marker_offset(i) == -1), "inconsistency"); - value->as_InlineType()->store_flat(kit, base, ptr, holder, offset, field_null_marker_offset(i), decorators); + value->as_InlineType()->store_flat(kit, base, ptr, holder, offset, holder_offset + field_null_marker_offset(i), decorators); } else { // Store field value to memory const TypePtr* adr_type = field_adr_type(base, offset, holder, decorators, kit->gvn()); diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index 7cb38a500d1..000584f5dce 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -1514,7 +1514,9 @@ static int compare(ReassignedField* left, ReassignedField* right) { // returned by HotSpotResolvedObjectTypeImpl.getInstanceFields(true) static int reassign_fields_by_klass(InstanceKlass* klass, frame* fr, RegisterMap* reg_map, ObjectValue* sv, int svIndex, oop obj, bool skip_internal, int base_offset, GrowableArray* null_markers, TRAPS) { GrowableArray* fields = new GrowableArray(); + bool doIt = false; if (null_markers == nullptr) { + doIt = true; // Null markers are no real fields.. null_markers = new GrowableArray(); } @@ -1638,11 +1640,12 @@ static int reassign_fields_by_klass(InstanceKlass* klass, frame* fr, RegisterMap } svIndex++; } - if (base_offset == 0) { + if (doIt) { for (int i = 0; i < null_markers->length(); ++i) { int offset = null_markers->at(i)._offset; + tty->print_cr("Null marker %d %d", offset, svIndex); jbyte is_init = (jbyte)StackValue::create_stack_value(fr, reg_map, sv->field_at(svIndex++))->get_jint(); - tty->print_cr("NULL MARKER %d %d", offset, is_init); + tty->print_cr("Value %d", is_init); obj->byte_field_put(offset, is_init); } } diff --git a/src/hotspot/share/runtime/signature.cpp b/src/hotspot/share/runtime/signature.cpp index 43640faae66..d51d1f5da12 100644 --- a/src/hotspot/share/runtime/signature.cpp +++ b/src/hotspot/share/runtime/signature.cpp @@ -687,7 +687,7 @@ ssize_t SignatureVerifier::is_valid_type(const char* type, ssize_t limit) { #endif // ASSERT // Adds an argument to the signature -void SigEntry::add_entry(GrowableArray* sig, BasicType bt, Symbol* symbol, int offset, int sort_offset) { +void SigEntry::add_entry(GrowableArray* sig, BasicType bt, Symbol* symbol, int offset, float sort_offset) { if (sort_offset == -1) { sort_offset = offset; } diff --git a/src/hotspot/share/runtime/signature.hpp b/src/hotspot/share/runtime/signature.hpp index 28133bf6da9..0ab0ef19fa9 100644 --- a/src/hotspot/share/runtime/signature.hpp +++ b/src/hotspot/share/runtime/signature.hpp @@ -580,13 +580,13 @@ class SigEntry { // TODO improve these comments BasicType _bt; int _offset; // Offset of the corresponding field in it's value class holder for scalarized arguments (-1 otherwise). Used for packing and unpacking - int _sort_offset; // Offset used for sorting + float _sort_offset; // Offset used for sorting Symbol* _symbol; // For printing SigEntry() : _bt(T_ILLEGAL), _offset(-1), _sort_offset(-1), _symbol(NULL) {} - SigEntry(BasicType bt, int offset = -1, int sort_offset = -1, Symbol* symbol = nullptr) + SigEntry(BasicType bt, int offset = -1, float sort_offset = -1, Symbol* symbol = nullptr) : _bt(bt), _offset(offset), _sort_offset(sort_offset), _symbol(symbol) {} static int compare(SigEntry* e1, SigEntry* e2) { @@ -614,7 +614,7 @@ class SigEntry { ShouldNotReachHere(); return 0; } - static void add_entry(GrowableArray* sig, BasicType bt, Symbol* symbol = nullptr, int offset = -1, int sort_offset = -1); + static void add_entry(GrowableArray* sig, BasicType bt, Symbol* symbol = nullptr, int offset = -1, float sort_offset = -1); static bool skip_value_delimiters(const GrowableArray* sig, int i); static int fill_sig_bt(const GrowableArray* sig, BasicType* sig_bt); static TempNewSymbol create_symbol(const GrowableArray* sig);