[aarch64-port-dev ] Use an explicit set of registers rather than a bitmap for psh and pop operations.

Andrew Haley aph at redhat.com
Tue Apr 22 18:10:47 UTC 2014


The code using push() and pop() was getting to be very hard to understand.

This patch defines a set of registers as a type, transforming

	pop(0x3fffffff, sp);

into

	pop(RegSet::range(r0, r29), sp);

This is fairly delicate code, so I'd appreciate someone going
over it to check that I haven't made mistakes.

Andrew.
-------------- next part --------------
# HG changeset patch
# User aph
# Date 1398189279 -3600
#      Tue Apr 22 18:54:39 2014 +0100
# Node ID 4c3b20781d5d4e187662896d4957e28aa332973b
# Parent  d9468835bc5160b7fac6709b0afbc751b2159fbb
Use an explicit set of registers rather than a bitmap for psh and pop operations.

diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/c1_CodeStubs_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_CodeStubs_aarch64.cpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/c1_CodeStubs_aarch64.cpp	Tue Apr 22 18:54:39 2014 +0100
@@ -54,7 +54,7 @@
 
   __ enter();
   __ sub(sp, sp, 2 * wordSize);
-  __ push(0x3fffffff, sp);         // integer registers except lr & sp
+  __ push(RegSet::range(r0, r29), sp);         // integer registers except lr & sp
   for (int i = 30; i >= 0; i -= 2) // caller-saved fp registers
     if (i < 8 || i > 15)
       __ stpd(as_FloatRegister(i), as_FloatRegister(i+1),
@@ -103,7 +103,7 @@
     if (i < 8 || i > 15)
       __ ldpd(as_FloatRegister(i), as_FloatRegister(i+1),
 	      Address(__ post(sp, 2 * wordSize)));
-  __ pop(0x3fffffff, sp);
+  __ pop(RegSet::range(r0, r29), sp);
 
   __ ldr(as_reg(result()), Address(rfp, -wordSize));
   __ leave();
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp	Tue Apr 22 18:54:39 2014 +0100
@@ -69,7 +69,7 @@
   int call_offset = offset();
   // verify callee-saved register
 #ifdef ASSERT
-  push(0b1, sp); // r0
+  push(RegSet::of(r0), sp);
   { Label L;
     get_thread(r0);
     cmp(rthread, r0);
@@ -77,7 +77,7 @@
     stop("StubAssembler::call_RT: rthread not callee saved?");
     bind(L);
   }
-  pop(0b1, sp);
+  pop(RegSet::of(r0), sp);
 #endif
   reset_last_Java_frame(true, true);
 
@@ -262,7 +262,7 @@
                                    bool save_fpu_registers = true) {
   __ block_comment("save_live_registers");
 
-  __ push(0x3fffffff, sp);         // integer registers except lr & sp
+  __ push(RegSet::range(r0, r29), sp);         // integer registers except lr & sp
 
   if (save_fpu_registers) {
     for (int i = 30; i >= 0; i -= 2)
@@ -284,7 +284,7 @@
     __ add(sp, sp, 32 * wordSize);
   }
 
-  __ pop(0x3fffffff, sp);
+  __ pop(RegSet::range(r0, r29), sp);
 }
 
 static void restore_live_registers_except_r0(StubAssembler* sasm, bool restore_fpu_registers = true)  {
@@ -298,7 +298,7 @@
   }
 
   __ ldp(zr, r1, Address(__ post(sp, 16)));
-  __ pop(0x3ffffffc, sp);
+  __ pop(RegSet::range(r2, r29), sp);
 }
 
 
@@ -1014,7 +1014,7 @@
         };
 
         __ set_info("slow_subtype_check", dont_gc_arguments);
-	__ push((1 << 0) | (1 <<2) | (1 << 4) | (1 << 5), sp);
+	__ push(RegSet::of(r0, r2, r4, r5), sp);
 
         // This is called by pushing args and not with C abi
         // __ ldr(r4, Address(sp, (klass_off) * VMRegImpl::stack_slot_size)); // subclass
@@ -1028,12 +1028,12 @@
         // fallthrough on success:
 	__ mov(rscratch1, 1);
         __ str(rscratch1, Address(sp, (result_off) * VMRegImpl::stack_slot_size)); // result
-	__ pop((1 << 0) | (1 <<2) | (1 << 4) | (1 << 5), sp);
+	__ pop(RegSet::of(r0, r2, r4, r5), sp);
         __ ret(lr);
 
         __ bind(miss);
         __ str(zr, Address(sp, (result_off) * VMRegImpl::stack_slot_size)); // result
-	__ pop((1 << 0) | (1 <<2) | (1 << 4) | (1 << 5), sp);
+	__ pop(RegSet::of(r0, r2, r4, r5), sp);
         __ ret(lr);
       }
       break;
@@ -1154,12 +1154,7 @@
 #if INCLUDE_ALL_GCS
 
 // Registers to be saved around calls to g1_wb_pre or g1_wb_post
-#define G1_SAVE_REGS  ((r0->bit(1)|r1->bit(1)| \
-			r2->bit(1)|r3->bit(1)|r4->bit(1)|r5->bit(1)| \
-			r6->bit(1)|r7->bit(1)|r8->bit(1)|r9->bit(1)| \
-			r10->bit(1)|r11->bit(1)|r12->bit(1)|r13->bit(1)| \
-			r14->bit(1)|r15->bit(1)|r16->bit(1)|r17->bit(1)| \
-			r18->bit(1))&~(rscratch1->bit(1)|rscratch2->bit(1)))
+#define G1_SAVE_REGS (RegSet::range(r0, r18) - RegSet::of(rscratch1, rscratch2))
 
     case g1_pre_barrier_slow_id:
       {
@@ -1262,10 +1257,10 @@
 
         const Register buffer_addr = r0;
 
-	__ push(r0->bit(1) | r1->bit(1), sp);
+	__ push(RegSet::of(r0, r1), sp);
 	__ ldr(buffer_addr, buffer);
 	__ str(card_addr, Address(buffer_addr, rscratch1));
-	__ pop(r0->bit(1) | r1->bit(1), sp);
+	__ pop(RegSet::of(r0, r1), sp);
 	__ b(done);
 
         __ bind(runtime);
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/interp_masm_aarch64.hpp
--- a/src/cpu/aarch64/vm/interp_masm_aarch64.hpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/interp_masm_aarch64.hpp	Tue Apr 22 18:54:39 2014 +0100
@@ -146,8 +146,8 @@
   void pop(TosState state); // transition vtos -> state
   void push(TosState state); // transition state -> vtos
 
-  void pop(unsigned bitset, Register stack) { ((MacroAssembler*)this)->pop(bitset, stack); }
-  void push(unsigned bitset, Register stack) { ((MacroAssembler*)this)->push(bitset, stack); }
+  void pop(RegSet regs, Register stack) { ((MacroAssembler*)this)->pop(regs, stack); }
+  void push(RegSet regs, Register stack) { ((MacroAssembler*)this)->push(regs, stack); }
 
   void empty_expression_stack() {
     ldr(esp, Address(rfp, frame::interpreter_frame_monitor_block_top_offset * wordSize));
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/macroAssembler_aarch64.hpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.hpp	Tue Apr 22 18:54:39 2014 +0100
@@ -403,9 +403,15 @@
   void mov_immediate64(Register dst, u_int64_t imm64);
   void mov_immediate32(Register dst, u_int32_t imm32);
 
+  int push(unsigned int bitset, Register stack);
+  int pop(unsigned int bitset, Register stack);
+
+public:
+  int push(RegSet regs, Register stack) { if (regs.bits()) push(regs.bits(), stack); }
+  int pop(RegSet regs, Register stack) { if (regs.bits()) pop(regs.bits(), stack); }
+
   // now mov instructions for loading absolute addresses and 32 or
   // 64 bit integers
-public:
 
   inline void mov(Register dst, address addr)
   {
@@ -1229,9 +1235,6 @@
   void pusha();
   void popa();
 
-  int push(unsigned int bitset, Register stack);
-  int pop(unsigned int bitset, Register stack);
-
   void repne_scan(Register addr, Register value, Register count,
 		  Register scratch);
   void repne_scanw(Register addr, Register value, Register count,
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/methodHandles_aarch64.cpp
--- a/src/cpu/aarch64/vm/methodHandles_aarch64.cpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/methodHandles_aarch64.cpp	Tue Apr 22 18:54:39 2014 +0100
@@ -77,7 +77,7 @@
   BLOCK_COMMENT("verify_klass {");
   __ verify_oop(obj);
   __ cbz(obj, L_bad);
-  __ push(temp->bit() | temp2->bit(), sp);
+  __ push(RegSet::of(temp, temp2), sp);
   __ load_klass(temp, obj);
   __ cmpptr(temp, ExternalAddress((address) klass_addr));
   __ br(Assembler::EQ, L_ok);
@@ -85,11 +85,11 @@
   __ ldr(temp, Address(temp, super_check_offset));
   __ cmpptr(temp, ExternalAddress((address) klass_addr));
   __ br(Assembler::EQ, L_ok);
-  __ pop(temp->bit() | temp2->bit(), sp);
+  __ pop(RegSet::of(temp, temp2), sp);
   __ bind(L_bad);
   __ stop(error_message);
   __ BIND(L_ok);
-  __ pop(temp->bit() | temp2->bit(), sp);
+  __ pop(RegSet::of(temp, temp2), sp);
   BLOCK_COMMENT("} verify_klass");
 }
 
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/register_aarch64.hpp
--- a/src/cpu/aarch64/vm/register_aarch64.hpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/register_aarch64.hpp	Tue Apr 22 18:54:39 2014 +0100
@@ -232,4 +232,57 @@
   static const int max_fpr;
 };
 
+// A set of registers
+class RegSet {
+  uint32_t _bitset;
+
+  RegSet(uint32_t bitset) : _bitset (bitset) { }
+
+public:
+
+  RegSet() : _bitset(0) { }
+
+  RegSet operator+(Register r1) const {
+    RegSet result(_bitset | r1->bit());
+    return result;
+  }
+
+  RegSet operator+(RegSet aSet) const {
+    RegSet result(_bitset | aSet._bitset);
+    return result;
+  }
+
+  RegSet operator-(RegSet aSet) const {
+    RegSet result(_bitset & ~aSet._bitset);
+    return result;
+  }
+
+  static RegSet of(Register r1) {
+    return RegSet(r1->bit());
+  }
+
+  static RegSet of(Register r1, Register r2) {
+    return of(r1) + r2;
+  }
+
+  static RegSet of(Register r1, Register r2, Register r3) {
+    return of(r1, r2) + r3;
+  }
+
+  static RegSet of(Register r1, Register r2, Register r3, Register r4) {
+    return of(r1, r2, r3) + r4;
+  }
+
+  static RegSet range(Register start, Register end) {
+    uint32_t bits = ~0;
+    bits <<= start->encoding();
+    bits <<= 31 - end->encoding();
+    bits >>= 31 - end->encoding();
+
+    return RegSet(bits);
+  }
+
+  uint32_t bits() const { return _bitset; }
+};
+
 #endif // CPU_AARCH64_VM_REGISTER_AARCH64_HPP
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp
--- a/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp	Tue Apr 22 18:54:39 2014 +0100
@@ -1052,29 +1052,27 @@
   }
 }
 static void save_args(MacroAssembler *masm, int arg_count, int first_arg, VMRegPair *args) {
-  unsigned long x = 0;  // Register bit vector
+  RegSet x;
   for ( int i = first_arg ; i < arg_count ; i++ ) {
     if (args[i].first()->is_Register()) {
-      x |= 1 << args[i].first()->as_Register()->encoding();
+      x = x + args[i].first()->as_Register();
     } else if (args[i].first()->is_FloatRegister()) {
       __ strd(args[i].first()->as_FloatRegister(), Address(__ pre(sp, -2 * wordSize)));
     }
   }
-  if (x)
-    __ push(x, sp);
+  __ push(x, sp);
 }
 
 static void restore_args(MacroAssembler *masm, int arg_count, int first_arg, VMRegPair *args) {
-  unsigned long x = 0;
+  RegSet x;
   for ( int i = first_arg ; i < arg_count ; i++ ) {
     if (args[i].first()->is_Register()) {
-      x |= 1 << args[i].first()->as_Register()->encoding();
+      x = x + args[i].first()->as_Register();
     } else {
       ;
     }
   }
-  if (x)
-    __ pop(x, sp);
+  __ pop(x, sp);
   for ( int i = first_arg ; i < arg_count ; i++ ) {
     if (args[i].first()->is_Register()) {
       ;
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
--- a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp	Tue Apr 22 18:54:39 2014 +0100
@@ -805,7 +805,7 @@
     __ bind(error);
     __ ldp(c_rarg3, c_rarg2, Address(__ post(sp, 16)));
 
-    __ push(0x7fffffff, sp);
+    __ push(RegSet::range(r0, r29), sp);
     // debug(char* msg, int64_t pc, int64_t regs[])
     __ ldr(c_rarg0, Address(sp, rscratch1->encoding()));    // pass address of error message
     __ mov(c_rarg1, Address(sp, lr));             // pass return address
@@ -816,7 +816,7 @@
     BLOCK_COMMENT("call MacroAssembler::debug");
     __ mov(rscratch1, CAST_FROM_FN_PTR(address, MacroAssembler::debug64));
     __ blrt(rscratch1, 3, 0, 1);
-    __ pop(0x7fffffff, sp);
+    __ pop(RegSet::range(r0, r29), sp);
 
     __ ldp(rscratch2, lr, Address(__ post(sp, 2 * wordSize)));
     __ ldp(r0, rscratch1, Address(__ post(sp, 2 * wordSize)));
@@ -880,7 +880,7 @@
     case BarrierSet::G1SATBCTLogging:
       // With G1, don't generate the call if we statically know that the target in uninitialized
       if (!dest_uninitialized) {
-	__ push(0x3fffffff, sp);         // integer registers except lr & sp
+	__ push(RegSet::range(r0, r29), sp);         // integer registers except lr & sp
 	if (count == c_rarg0) {
 	  if (addr == c_rarg1) {
 	    // exactly backwards!!
@@ -895,7 +895,7 @@
 	  __ mov(c_rarg1, count);
 	}
 	__ call_VM_leaf(CAST_FROM_FN_PTR(address, BarrierSet::static_write_ref_array_pre), 2);
-	__ pop(0x3fffffff, sp);         // integer registers except lr & sp        }
+	__ pop(RegSet::range(r0, r29), sp);         // integer registers except lr & sp        }
 	break;
       case BarrierSet::CardTableModRef:
       case BarrierSet::CardTableExtension:
@@ -926,7 +926,7 @@
       case BarrierSet::G1SATBCTLogging:
 
         {
-	  __ push(0x3fffffff, sp);         // integer registers except lr & sp
+	  __ push(RegSet::range(r0, r29), sp);         // integer registers except lr & sp
           // must compute element count unless barrier set interface is changed (other platforms supply count)
           assert_different_registers(start, end, scratch);
           __ lea(scratch, Address(end, BytesPerHeapOop));
@@ -935,7 +935,7 @@
           __ mov(c_rarg0, start);
           __ mov(c_rarg1, scratch);
           __ call_VM_leaf(CAST_FROM_FN_PTR(address, BarrierSet::static_write_ref_array_post), 2);
-	  __ pop(0x3fffffff, sp);         // integer registers except lr & sp        }
+	  __ pop(RegSet::range(r0, r29), sp);         // integer registers except lr & sp        }
         }
         break;
       case BarrierSet::CardTableModRef:
@@ -1298,13 +1298,13 @@
     }
     __ enter();
     if (is_oop) {
-      __ push(d->bit() | count->bit(), sp);
+      __ push(RegSet::of(d, count), sp);
       // no registers are destroyed by this call
       gen_write_ref_array_pre_barrier(d, count, dest_uninitialized);
     }
     copy_memory(aligned, s, d, count, rscratch1, size);
     if (is_oop) {
-      __ pop(d->bit() | count->bit(), sp);
+      __ pop(RegSet::of(d, count), sp);
       if (VerifyOops)
 	verify_oop_array(size, d, count, r16);
       __ sub(count, count, 1); // make an inclusive end pointer
@@ -1350,13 +1350,13 @@
 
     __ enter();
     if (is_oop) {
-      __ push(d->bit() | count->bit(), sp);
+      __ push(RegSet::of(d, count), sp);
       // no registers are destroyed by this call
       gen_write_ref_array_pre_barrier(d, count, dest_uninitialized);
     }
     copy_memory(aligned, s, d, count, rscratch1, -size);
     if (is_oop) {
-      __ pop(d->bit() | count->bit(), sp);
+      __ pop(RegSet::of(d, count), sp);
       if (VerifyOops)
 	verify_oop_array(size, d, count, r16);
       __ sub(count, count, 1); // make an inclusive end pointer
@@ -1682,7 +1682,7 @@
      // Empty array:  Nothing to do.
     __ cbz(count, L_done);
 
-    __ push(r18->bit() | r19->bit() | r20->bit() | r21->bit(), sp);
+    __ push(RegSet::of(r18, r19, r20, r21), sp);
 
 #ifdef ASSERT
     BLOCK_COMMENT("assert consistent ckoff/ckval");
@@ -1743,7 +1743,7 @@
     gen_write_ref_array_post_barrier(start_to, to, rscratch1);
 
     __ bind(L_done_pop);
-    __ pop(r18->bit() | r19->bit() | r20->bit()| r21->bit(), sp);
+    __ pop(RegSet::of(r18, r19, r20, r21), sp);
     inc_counter_np(SharedRuntime::_checkcast_array_copy_ctr);
 
     __ bind(L_done);
diff -r d9468835bc51 -r 4c3b20781d5d src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp
--- a/src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp	Thu Apr 10 06:50:43 2014 -0400
+++ b/src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp	Tue Apr 22 18:54:39 2014 +0100
@@ -1810,12 +1810,12 @@
 
   __ push(lr);
   __ push(state);
-  __ push(0xffffu, sp);
+  __ push(RegSet::range(r0, r15), sp);
   __ mov(c_rarg2, r0);  // Pass itos
   __ call_VM(noreg,
              CAST_FROM_FN_PTR(address, SharedRuntime::trace_bytecode),
              c_rarg1, c_rarg2, c_rarg3);
-  __ pop(0xffffu, sp);
+  __ pop(RegSet::range(r0, r15), sp);
   __ pop(state);
   __ pop(lr);
   __ ret(lr);                                   // return from result handler


More information about the aarch64-port-dev mailing list