[aarch64-port-dev ] Fixes for compressed OOPs and VerifyOops

Andrew Haley aph at redhat.com
Thu Sep 12 06:33:08 PDT 2013


We had a couple of bugs in compressed OOPs and VerifyOops.

VerifyOops didn't really work at all, so this is a substantial rewrite.

Andrew.


# HG changeset patch
# User aph
# Date 1378921822 -3600
# Node ID a9f01f39ab447c9d32395c77a770381da22a407d
# Parent  3c1ff2b85f84f7d8fb6adb4ad3739c43cd20fe67
Fixes for compressed OOPs and VerifyOops

diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	Wed Sep 11 18:50:22 2013 +0100
@@ -2763,18 +2763,6 @@
       }
   } else if (src->is_address()) {
     LIR_Address* from_addr = src->as_address_ptr();
-    Register compressed_dest = noreg;
-    if (is_reg(dest)) {
-      compressed_dest = as_reg(dest);
-      if (type == T_ARRAY || type == T_OBJECT) {
-	__ verify_oop(dest->as_register());
-	if (UseCompressedOops) {
-	  compressed_dest = rscratch2;
-	  __ mov(compressed_dest, as_reg(dest));
-	  __ encode_heap_oop(compressed_dest);
-	}
-      }
-    }

     if (src->is_double_cpu())
       __ lea(rscratch1, as_Address(from_addr));
diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.cpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/c1_MacroAssembler_aarch64.cpp	Wed Sep 11 18:50:22 2013 +0100
@@ -190,7 +190,7 @@
   str(t1, Address(obj, oopDesc::mark_offset_in_bytes()));

   if (UseCompressedKlassPointers) { // Take care not to kill klass
-    encode_heap_oop_not_null(t1, klass);
+    encode_klass_not_null(t1, klass);
     strw(t1, Address(obj, oopDesc::klass_offset_in_bytes()));
   } else {
     str(klass, Address(obj, oopDesc::klass_offset_in_bytes()));
@@ -466,7 +466,10 @@

 #ifndef PRODUCT

-void C1_MacroAssembler::verify_stack_oop(int stack_offset) { Unimplemented(); }
+void C1_MacroAssembler::verify_stack_oop(int stack_offset) {
+  if (!VerifyOops) return;
+  verify_oop_addr(Address(sp, stack_offset), "oop");
+}

 void C1_MacroAssembler::verify_not_null_oop(Register r) {
   if (!VerifyOops) return;
diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Wed Sep 11 18:50:22 2013 +0100
@@ -850,31 +850,68 @@
 void MacroAssembler::verify_oop(Register reg, const char* s) {
   if (!VerifyOops) return;

-  assert(reg != rmethod, "bad reg in verify_oop");
-
   // Pass register number to verify_oop_subroutine
   char* b = new char[strlen(s) + 50];
   sprintf(b, "verify_oop: %s: %s", reg->name(), s);
   BLOCK_COMMENT("verify_oop {");
-  stp(r0, rscratch1, Address(pre(sp, -16)));
+  stp(r0, rscratch1, Address(pre(sp, -2 * wordSize)));

   ExternalAddress buffer((address) b);
   // our contract is not to modify anything
   mov(rscratch1, buffer.target());
-  stp(rscratch1, reg, Address(pre(sp, -16)));
+  stp(rscratch1, reg, Address(pre(sp, -2 * wordSize)));

   // call indirectly to solve generation ordering problem
-  stp(reg, lr, Address(pre(sp, -16)));
+  stp(reg, lr, Address(pre(sp, -2 * wordSize)));
   lea(rscratch1, ExternalAddress(StubRoutines::verify_oop_subroutine_entry_address()));
   ldr(rscratch1, Address(rscratch1));
   blr(rscratch1);
-  ldp(reg, lr, Address(post(sp, 16)));
+  ldp(reg, lr, Address(post(sp, 2 * wordSize)));
   add(sp, sp, 2 * wordSize);
-  ldp(r0, rscratch1, Address(post(sp, 16)));
+  ldp(r0, rscratch1, Address(post(sp, 2 * wordSize)));

   BLOCK_COMMENT("} verify_oop");
 }

+void MacroAssembler::verify_oop_addr(Address addr, const char* s) {
+  return;
+  if (!VerifyOops) return;
+
+  // Pass register number to verify_oop_subroutine
+  char* b = new char[strlen(s) + 50];
+  sprintf(b, "verify_oop_addr: %s", s);
+
+  BLOCK_COMMENT("verify_oop_addr {");
+
+  stp(r0, rscratch1, Address(pre(sp, -2 * wordSize)));
+  // addr may contain rsp so we will have to adjust it based on the push
+  // we just did (and on 64 bit we do two pushes)
+  // NOTE: 64bit seemed to have had a bug in that it did movq(addr, r0); which
+  // stores r0 into addr which is backwards of what was intended.
+  if (addr.uses(sp)) {
+    lea(r0, addr);
+    ldr(r0, Address(r0, 2 * wordSize));
+  } else {
+    ldr(r0, addr);
+  }
+
+  ExternalAddress buffer((address) b);
+  // our contract is not to modify anything
+  mov(rscratch1, buffer.target());
+  stp(rscratch1, r0, Address(pre(sp, -2 * wordSize)));
+
+  // call indirectly to solve generation ordering problem
+  stp(zr, lr, Address(pre(sp, -2 * wordSize)));
+  lea(rscratch1, ExternalAddress(StubRoutines::verify_oop_subroutine_entry_address()));
+  ldr(rscratch1, Address(rscratch1));
+  blr(rscratch1);
+  ldp(zr, lr, Address(post(sp, 2 * wordSize)));
+  add(sp, sp, 2 * wordSize);
+  ldp(r0, rscratch1, Address(post(sp, 2 * wordSize)));
+
+  BLOCK_COMMENT("} verify_oop_addr");
+}
+
 Address MacroAssembler::argument_address(RegisterOrConstant arg_slot,
                                          int extra_slot_offset) {
   // cf. TemplateTable::prepare_invoke(), if (load_receiver).
@@ -2176,7 +2213,7 @@
       add(dst, zr, src, Assembler::LSL, LogKlassAlignmentInBytes);
     }
   } else {
-    assert (Universe::narrow_oop_base() == NULL, "sanity");
+    assert (Universe::narrow_klass_base() == NULL, "sanity");
     if (dst != src) {
       mov(dst, src);
     }
@@ -2514,28 +2551,26 @@
 #ifdef ASSERT
   if (UseTLAB && VerifyOops) {
     Label next, ok;
-    Register t1 = r4;

-    str(t1, Address(pre(sp, -16)));
-    push(t1);
+    stp(rscratch2, rscratch1, Address(pre(sp, -16)));

-    ldr(t1, Address(rthread, in_bytes(JavaThread::tlab_top_offset())));
+    ldr(rscratch2, Address(rthread, in_bytes(JavaThread::tlab_top_offset())));
     ldr(rscratch1, Address(rthread, in_bytes(JavaThread::tlab_start_offset())));
-    cmp(t1, rscratch1);
+    cmp(rscratch2, rscratch1);
     br(Assembler::HS, next);
     STOP("assert(top >= start)");
     should_not_reach_here();

     bind(next);
-    ldr(t1, Address(rthread, in_bytes(JavaThread::tlab_end_offset())));
+    ldr(rscratch2, Address(rthread, in_bytes(JavaThread::tlab_end_offset())));
     ldr(rscratch1, Address(rthread, in_bytes(JavaThread::tlab_top_offset())));
-    cmp(t1, rscratch1);
+    cmp(rscratch2, rscratch1);
     br(Assembler::HS, ok);
     STOP("assert(top <= end)");
     should_not_reach_here();

     bind(ok);
-    ldr(t1, Address(post(sp, 16)));
+    ldp(rscratch2, rscratch1, Address(post(sp, 16)));
   }
 #endif
 }
diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/macroAssembler_aarch64.hpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp	Wed Sep 11 18:50:22 2013 +0100
@@ -937,10 +937,7 @@

   // only if +VerifyOops
   void verify_oop(Register reg, const char* s = "broken oop");
-  // unimplemented
-#if 0
   void verify_oop_addr(Address addr, const char * s = "broken oop addr");
-#endif

 // TODO: verify method and klass metadata (compare against vptr?)
   void _verify_method_ptr(Register reg, const char * msg, const char * file, int line) {}
@@ -1373,6 +1370,7 @@

   typedef void (Assembler::* flush_insn)(Register Rt);
   void generate_flush_loop(flush_insn flush, Register start, Register lines);
+
 };

 #ifdef ASSERT
diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/methodHandles_aarch64.cpp
--- a/src/cpu/aarch64/vm/methodHandles_aarch64.cpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/methodHandles_aarch64.cpp	Wed Sep 11 18:50:22 2013 +0100
@@ -102,13 +102,6 @@
   assert(method == rmethod, "interpreter calling convention");
   __ verify_method_ptr(method);

-#ifndef PRODUCT
-  // tell the simulator that the method has been entered
-  if (NotifySimulator) {
-    __ notify(Assembler::method_entry);
-  }
-#endif
-
   if (!for_compiler_entry && JvmtiExport::can_post_interpreter_events()) {
     Label run_compiled_code;
     // JVMTI events, such as single-stepping, are implemented partly by avoiding running
diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp
--- a/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp	Wed Sep 11 18:50:22 2013 +0100
@@ -1188,7 +1188,7 @@
         VMReg r = regs[i].first();
         assert(r->is_valid(), "bad oop arg");
         if (r->is_stack()) {
-          __ ldr(temp_reg, Address(sp, r->reg2stack() * VMRegImpl::stack_slot_size + wordSize));
+          __ ldr(temp_reg, Address(sp, r->reg2stack() * VMRegImpl::stack_slot_size));
           __ verify_oop(temp_reg);
         } else {
           __ verify_oop(r->as_Register());
diff -r 3c1ff2b85f84 -r a9f01f39ab44 src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
--- a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp	Wed Sep 11 13:32:05 2013 +0100
+++ b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp	Wed Sep 11 18:50:22 2013 +0100
@@ -713,19 +713,24 @@
   //  * [tos + 7]: saved rscratch1 - saved by caller
   //  * = popped on exit
   address generate_verify_oop() {
+
     StubCodeMark mark(this, "StubRoutines", "verify_oop");
     address start = __ pc();

     Label exit, error;

     // __ pushf();
-    // __ incrementl(ExternalAddress((address) StubRoutines::verify_oop_count_addr()));
-
     // __ push(r12);

     // save c_rarg2 and c_rarg3
     __ stp(c_rarg3, c_rarg2, Address(__ pre(sp, -16)));

+    // __ incrementl(ExternalAddress((address) StubRoutines::verify_oop_count_addr()));
+    __ lea(c_rarg2, ExternalAddress((address) StubRoutines::verify_oop_count_addr()));
+    __ ldr(c_rarg3, Address(c_rarg2));
+    __ add(c_rarg3, c_rarg3, 1);
+    __ str(c_rarg3, Address(c_rarg2));
+
     enum {
            // After previous pushes.
            oop_to_verify = 5 * wordSize,
@@ -743,12 +748,14 @@
     __ cbz(r0, exit); // if obj is NULL it is OK

     // Check if the oop is in the right area of memory
-    __ mov(c_rarg2, r0);
     __ mov(c_rarg3, (intptr_t) Universe::verify_oop_mask());
-    __ andr(c_rarg2, c_rarg2, c_rarg3);
+    __ andr(c_rarg2, r0, c_rarg3);
     __ mov(c_rarg3, (intptr_t) Universe::verify_oop_bits());
-    __ cmp(c_rarg2, c_rarg3);
-    __ br(Assembler::NE, error);
+
+    // Compare c_rarg2 and c_rarg3.  We don't use a compare
+    // instruction here because the flags register is live.
+    __ eor(c_rarg2, c_rarg2, c_rarg3);
+    __ cbnz(c_rarg2, error);

     // make sure klass is 'reasonable', which is not zero.
     __ load_klass(r0, r0);  // get klass
@@ -757,6 +764,7 @@

     // return if everything seems ok
     __ bind(exit);
+
     __ ldp(c_rarg3, c_rarg2, Address(__ post(sp, 16)));
     __ ret(lr);

@@ -791,6 +799,7 @@
     __ mov(sp, r19);                               // restore rsp
     __ pop(0x7fffffff, sp);
     __ ldr(lr, Address(sp, 0));
+
     __ ret(lr);

     return start;



More information about the aarch64-port-dev mailing list