8367564: Fields access in interpreter needs cleanup and refactoring

Reviewed-by: coleenp

Author: fparain

src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp

@@ -225,12 +225,16 @@ void InterpreterMacroAssembler::allocate_instance(Register klass, Register new_o
 void InterpreterMacroAssembler::read_flat_field(Register entry,
                                                 Register field_index, Register field_offset,
                                                 Register temp, Register obj) {
-  Label alloc_failed, done;
+  Label failed_alloc, slow_path, done;
   const Register src = field_offset;
   const Register alloc_temp = r10;
   const Register dst_temp   = field_index;
   const Register layout_info = temp;
-  assert_different_registers(obj, entry, field_index, field_offset, temp, alloc_temp);
+  assert_different_registers(obj, entry, field_index, field_offset, temp, alloc_temp, rscratch1);
+
+  load_unsigned_byte(temp, Address(entry, in_bytes(ResolvedFieldEntry::flags_offset())));
+  // If the field is nullable, jump to slow path
+  tbz(temp, ResolvedFieldEntry::is_null_free_inline_type_shift, slow_path);
 
   // Grab the inline field klass
   ldr(rscratch1, Address(entry, in_bytes(ResolvedFieldEntry::field_holder_offset())));
@@ -241,7 +245,7 @@ void InterpreterMacroAssembler::read_flat_field(Register entry,
 
   // allocate buffer
   push(obj); // save holder
-  allocate_instance(field_klass, obj, alloc_temp, rscratch2, false, alloc_failed);
+  allocate_instance(field_klass, obj, alloc_temp, rscratch2, false, failed_alloc);
 
   // Have an oop instance buffer, copy into it
   payload_address(obj, dst_temp, field_klass);  // danger, uses rscratch1
@@ -253,15 +257,45 @@ void InterpreterMacroAssembler::read_flat_field(Register entry,
   pop(obj);
   b(done);
 
-  bind(alloc_failed);
+  bind(failed_alloc);
   pop(obj);
+  bind(slow_path);
   call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::read_flat_field),
           obj, entry);
 
   bind(done);
   membar(Assembler::StoreStore);
 }
 
+void InterpreterMacroAssembler::write_flat_field(Register entry, Register field_offset,
+                                                 Register tmp1, Register tmp2,
+                                                 Register obj) {
+  assert_different_registers(entry, field_offset, tmp1, tmp2, obj);
+  Label slow_path, done;
+
+  load_unsigned_byte(tmp1, Address(entry, in_bytes(ResolvedFieldEntry::flags_offset())));
+  test_field_is_not_null_free_inline_type(tmp1, noreg /* temp */, slow_path);
+
+  null_check(r0); // FIXME JDK-8341120
+
+  add(obj, obj, field_offset);
+
+  load_klass(tmp1, r0);
+  payload_address(r0, r0, tmp1);
+
+  Register layout_info = field_offset;
+  load_unsigned_short(tmp1, Address(entry, in_bytes(ResolvedFieldEntry::field_index_offset())));
+  ldr(tmp2, Address(entry, in_bytes(ResolvedFieldEntry::field_holder_offset())));
+  inline_layout_info(tmp2, tmp1, layout_info);
+
+  flat_field_copy(IN_HEAP, r0, obj, layout_info);
+  b(done);
+
+  bind(slow_path);
+  call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::write_flat_field), obj, r0, entry);
+  bind(done);
+}
+
 // Load object from cpool->resolved_references(index)
 void InterpreterMacroAssembler::load_resolved_reference_at_index(
                                            Register result, Register index, Register tmp) {

src/hotspot/cpu/aarch64/interp_masm_aarch64.hpp

@@ -156,7 +156,11 @@ class InterpreterMacroAssembler: public MacroAssembler {
   //   - assumes holder_klass and valueKlass field klass have both been resolved
   void read_flat_field(Register entry,
                        Register field_index, Register field_offset,
-                       Register temp, Register obj = r0);
+                       Register temp, Register obj);
+
+  void write_flat_field(Register entry, Register field_offset,
+                        Register tmp1, Register tmp2,
+                        Register obj);
 
   // Allocate value buffer in "obj" and read in flat element at the given index
   // NOTES:

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp

@@ -2772,51 +2772,23 @@ void TemplateTable::getfield_or_static(int byte_no, bool is_static, RewriteContr
   } else { // Valhalla
     if (is_static) {
       __ load_heap_oop(r0, field, rscratch1, rscratch2);
-      Label is_null_free_inline_type, uninitialized;
-      // Issue below if the static field has not been initialized yet
-      __ test_field_is_null_free_inline_type(flags, noreg /*temp*/, is_null_free_inline_type);
-        // field is not a null free inline type
-        __ push(atos);
-        __ b(Done);
-      // field is a null free inline type, must not return null even if uninitialized
-      __ bind(is_null_free_inline_type);
-        __ cbz(r0, uninitialized);
-          __ push(atos);
-          __ b(Done);
-        __ bind(uninitialized);
-          __ b(ExternalAddress(Interpreter::_throw_NPE_UninitializedField_entry));
+      __ push(atos);
+      __ b(Done);
     } else {
-      Label is_flat, nonnull, is_inline_type, has_null_marker, rewrite_inline;
-      __ test_field_is_null_free_inline_type(flags, noreg /*temp*/, is_inline_type);
-      __ test_field_has_null_marker(flags, noreg /*temp*/, has_null_marker);
-        // Non-inline field case
-        __ load_heap_oop(r0, field, rscratch1, rscratch2);
+      Label is_flat, rewrite_inline;
+      __ test_field_is_flat(flags, noreg /*temp*/, is_flat);
+      __ load_heap_oop(r0, field, rscratch1, rscratch2);
+      __ push(atos);
+      if (rc == may_rewrite) {
+        patch_bytecode(Bytecodes::_fast_agetfield, bc, r1);
+      }
+      __ b(Done);
+      __ bind(is_flat);
+        // field is flat (null-free or nullable with a null-marker)
+        __ mov(r0, obj);
+        __ read_flat_field(cache, field_index, off, inline_klass /* temp */, r0);
+        __ verify_oop(r0);
         __ push(atos);
-        if (rc == may_rewrite) {
-          patch_bytecode(Bytecodes::_fast_agetfield, bc, r1);
-        }
-        __ b(Done);
-      __ bind(is_inline_type);
-        __ test_field_is_flat(flags, noreg /* temp */, is_flat);
-         // field is not flat
-          __ load_heap_oop(r0, field, rscratch1, rscratch2);
-          __ cbnz(r0, nonnull);
-            __ b(ExternalAddress(Interpreter::_throw_NPE_UninitializedField_entry));
-          __ bind(nonnull);
-          __ verify_oop(r0);
-          __ push(atos);
-          __ b(rewrite_inline);
-        __ bind(is_flat);
-        // field is flat
-          __ mov(r0, obj);
-          __ read_flat_field(cache, field_index, off, inline_klass /* temp */, r0);
-          __ verify_oop(r0);
-          __ push(atos);
-          __ b(rewrite_inline);
-        __ bind(has_null_marker);
-          call_VM(r0, CAST_FROM_FN_PTR(address, InterpreterRuntime::read_nullable_flat_field), obj, cache);
-          __ verify_oop(r0);
-          __ push(atos);
       __ bind(rewrite_inline);
       if (rc == may_rewrite) {
         patch_bytecode(Bytecodes::_fast_vgetfield, bc, r1);
@@ -3057,52 +3029,35 @@ void TemplateTable::putfield_or_static(int byte_no, bool is_static, RewriteContr
      } else { // Valhalla
       __ pop(atos);
       if (is_static) {
-        Label is_inline_type;
-         __ test_field_is_not_null_free_inline_type(flags, noreg /* temp */, is_inline_type);
-         __ null_check(r0);
-         __ bind(is_inline_type);
+        Label is_nullable;
+         __ test_field_is_not_null_free_inline_type(flags, noreg /* temp */, is_nullable);
+         __ null_check(r0);  // FIXME JDK-8341120
+         __ bind(is_nullable);
          do_oop_store(_masm, field, r0, IN_HEAP);
          __ b(Done);
       } else {
-        Label is_inline_type, is_flat, has_null_marker, rewrite_not_inline, rewrite_inline;
-        __ test_field_is_null_free_inline_type(flags, noreg /*temp*/, is_inline_type);
-        __ test_field_has_null_marker(flags, noreg /*temp*/, has_null_marker);
-        // Not an inline type
+        Label null_free_reference, is_flat, rewrite_inline;
+        __ test_field_is_flat(flags, noreg /*temp*/, is_flat);
+        __ test_field_is_null_free_inline_type(flags, noreg /*temp*/, null_free_reference);
         pop_and_check_object(obj);
         // Store into the field
         // Clobbers: r10, r11, r3
         do_oop_store(_masm, field, r0, IN_HEAP);
-        __ bind(rewrite_not_inline);
         if (rc == may_rewrite) {
           patch_bytecode(Bytecodes::_fast_aputfield, bc, r19, true, byte_no);
         }
         __ b(Done);
         // Implementation of the inline type semantic
-        __ bind(is_inline_type);
-        __ null_check(r0);
-        __ test_field_is_flat(flags, noreg /*temp*/, is_flat);
-        // field is not flat
+        __ bind(null_free_reference);
+        __ null_check(r0);  // FIXME JDK-8341120
         pop_and_check_object(obj);
         // Store into the field
         // Clobbers: r10, r11, r3
         do_oop_store(_masm, field, r0, IN_HEAP);
         __ b(rewrite_inline);
         __ bind(is_flat);
-        __ load_field_entry(cache, index); // reload field entry (cache) because it was erased by tos_state
-        __ load_unsigned_short(index, Address(cache, in_bytes(ResolvedFieldEntry::field_index_offset())));
-        __ ldr(r2, Address(cache, in_bytes(ResolvedFieldEntry::field_holder_offset())));
-        __ inline_layout_info(r2, index, r6);
-        pop_and_check_object(obj);
-        __ load_klass(inline_klass, r0);
-        __ payload_address(r0, r0, inline_klass);
-        __ add(obj, obj, off);
-        // because we use InlineLayoutInfo, we need special value access code specialized for fields (arrays will need a different API)
-        __ flat_field_copy(IN_HEAP, r0, obj, r6);
-        __ b(rewrite_inline);
-        __ bind(has_null_marker);
-        assert_different_registers(r0, cache, r19);
-        pop_and_check_object(r19);
-        __ call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::write_nullable_flat_field), r19, r0, cache);
+        pop_and_check_object(r7);
+        __ write_flat_field(cache, off, r3, r6, r7);
         __ bind(rewrite_inline);
         if (rc == may_rewrite) {
           patch_bytecode(Bytecodes::_fast_vputfield, bc, r19, true, byte_no);
@@ -3323,29 +3278,18 @@ void TemplateTable::fast_storefield(TosState state)
   // access field
   switch (bytecode()) {
   case Bytecodes::_fast_vputfield:
-   {
+    {
       Label is_flat, has_null_marker, done;
-      __ test_field_has_null_marker(r5, noreg /* temp */, has_null_marker);
-      __ null_check(r0);
       __ test_field_is_flat(r5, noreg /* temp */, is_flat);
-      // field is not flat
+      __ null_check(r0);
       do_oop_store(_masm, field, r0, IN_HEAP);
       __ b(done);
       __ bind(is_flat);
-      // field is flat
       __ load_field_entry(r4, r5);
-      __ load_unsigned_short(r5, Address(r4, in_bytes(ResolvedFieldEntry::field_index_offset())));
-      __ ldr(r4, Address(r4, in_bytes(ResolvedFieldEntry::field_holder_offset())));
-      __ inline_layout_info(r4, r5, r6);
-      __ load_klass(r4, r0);
-      __ payload_address(r0, r0, r4);
-      __ lea(rscratch1, field);
-      __ flat_field_copy(IN_HEAP, r0, rscratch1, r6);
-      __ b(done);
-      __ bind(has_null_marker);
-      __ load_field_entry(r4, r1);
-      __ mov(r1, r2);
-      __ call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::write_nullable_flat_field), r1, r0, r4);
+      // Re-shuffle registers because of VM calls calling convention
+      __ mov(r19, r1);
+      __ mov(r7, r2);
+      __ write_flat_field(r4, r19, r6, r8, r7);
       __ bind(done);
     }
     break;
@@ -3444,27 +3388,11 @@ void TemplateTable::fast_accessfield(TosState state)
   switch (bytecode()) {
   case Bytecodes::_fast_vgetfield:
     {
-      Register index = r4, klass = r5, inline_klass = r6, tmp = r7;
-      Label is_flat, has_null_marker, nonnull, Done;
-      __ test_field_has_null_marker(r3, noreg /*temp*/, has_null_marker);
-      __ test_field_is_flat(r3, noreg /* temp */, is_flat);
-        // field is not flat
-        __ load_heap_oop(r0, field, rscratch1, rscratch2);
-        __ cbnz(r0, nonnull);
-          __ b(ExternalAddress(Interpreter::_throw_NPE_UninitializedField_entry));
-        __ bind(nonnull);
-        __ verify_oop(r0);
-        __ b(Done);
-      __ bind(is_flat);
+      Register index = r4, tmp = r7;
       // field is flat
-        __ load_unsigned_short(index, Address(r2, in_bytes(ResolvedFieldEntry::field_index_offset())));
-        __ read_flat_field(r2, index, r1, tmp /* temp */, r0);
-        __ verify_oop(r0);
-        __ b(Done);
-      __ bind(has_null_marker);
-        call_VM(r0, CAST_FROM_FN_PTR(address, InterpreterRuntime::read_nullable_flat_field), r0, r2);
-        __ verify_oop(r0);
-      __ bind(Done);
+      __ load_unsigned_short(index, Address(r2, in_bytes(ResolvedFieldEntry::field_index_offset())));
+      __ read_flat_field(r2, index, r1, tmp /* temp */, r0);
+      __ verify_oop(r0);
     }
     break;
   case Bytecodes::_fast_agetfield:

src/hotspot/cpu/x86/interp_masm_x86.cpp

@@ -1081,22 +1081,25 @@ void InterpreterMacroAssembler::allocate_instance(Register klass, Register new_o
 }
 
 void InterpreterMacroAssembler::read_flat_field(Register entry, Register tmp1, Register tmp2, Register obj) {
-  Label alloc_failed, done;
+  Label alloc_failed, slow_path, done;
   const Register alloc_temp = LP64_ONLY(rscratch1) NOT_LP64(rsi);
   const Register dst_temp   = LP64_ONLY(rscratch2) NOT_LP64(rdi);
   assert_different_registers(obj, entry, tmp1, tmp2, dst_temp, r8, r9);
 
-  // FIXME: code below could be re-written to better use InlineLayoutInfo data structure
-  // see aarch64 version
+  // If the field is nullable, jump to slow path
+  load_unsigned_byte(tmp1, Address(entry, in_bytes(ResolvedFieldEntry::flags_offset())));
+  testl(tmp1, 1 << ResolvedFieldEntry::is_null_free_inline_type_shift);
+  jcc(Assembler::equal, slow_path);
 
   // Grap the inline field klass
   const Register field_klass = tmp1;
   load_unsigned_short(tmp2, Address(entry, in_bytes(ResolvedFieldEntry::field_index_offset())));
+
   movptr(tmp1, Address(entry, ResolvedFieldEntry::field_holder_offset()));
   get_inline_type_field_klass(tmp1, tmp2, field_klass);
 
   // allocate buffer
-  push(obj);  // push object being read from     // FIXME spilling on stack could probably be avoided by using tmp2
+  push(obj);  // push object being read from
   allocate_instance(field_klass, obj, alloc_temp, dst_temp, false, alloc_failed);
 
   // Have an oop instance buffer, copy into it
@@ -1110,19 +1113,51 @@ void InterpreterMacroAssembler::read_flat_field(Register entry, Register tmp1, R
   lea(tmp2, Address(alloc_temp, tmp2));
   // call_VM_leaf, clobbers a few regs, save restore new obj
   push(obj);
-  // access_value_copy(IS_DEST_UNINITIALIZED, tmp2, dst_temp, field_klass);
   flat_field_copy(IS_DEST_UNINITIALIZED, tmp2, dst_temp, r8);
   pop(obj);
   jmp(done);
 
   bind(alloc_failed);
   pop(obj);
+  bind(slow_path);
   call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::read_flat_field),
           obj, entry);
   get_vm_result_oop(obj);
   bind(done);
 }
 
+void InterpreterMacroAssembler::write_flat_field(Register entry, Register tmp1, Register tmp2,
+                                                 Register obj, Register off, Register value) {
+  assert_different_registers(entry, tmp1, tmp2, obj, off, value);
+
+  Label slow_path, done;
+
+  load_unsigned_byte(tmp2, Address(entry, in_bytes(ResolvedFieldEntry::flags_offset())));
+  test_field_is_not_null_free_inline_type(tmp2, tmp1, slow_path);
+
+  null_check(value); // FIXME JDK-8341120
+
+  lea(obj, Address(obj, off, Address::times_1));
+
+  load_klass(tmp2, value, tmp1);
+  payload_addr(value, value, tmp2);
+
+  Register idx = tmp1;
+  load_unsigned_short(idx, Address(entry, in_bytes(ResolvedFieldEntry::field_index_offset())));
+  movptr(tmp2, Address(entry, in_bytes(ResolvedFieldEntry::field_holder_offset())));
+
+  Register layout_info = off;
+  inline_layout_info(tmp2, idx, layout_info);
+
+  flat_field_copy(IN_HEAP, value, obj, layout_info);
+  jmp(done);
+
+  bind(slow_path);
+  call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::write_flat_field), obj, value, entry);
+
+  bind(done);
+}
+
 // Lock object
 //
 // Args:

src/hotspot/cpu/x86/interp_masm_x86.hpp

@@ -215,6 +215,9 @@ class InterpreterMacroAssembler: public MacroAssembler {
   void read_flat_field(Register entry,
                        Register tmp1, Register tmp2,
                        Register obj = rax);
+  void write_flat_field(Register entry,
+                        Register tmp1, Register tmp2,
+                        Register obj, Register off, Register value);
 
   // Object locking
   void lock_object  (Register lock_reg);