A64: Tidy up Push and Pop TODOs.

src/a64/macro-assembler-a64.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 605 matching lines @@
 
   if (!csp.Is(StackPointer()) && emit_debug_code()) {
     // It is safe to leave csp where it is when unwinding the JavaScript stack,
     // but if we keep it matching StackPointer, the simulator can detect memory
     // accesses in the now-free part of the stack.
     Mov(csp, StackPointer());
   }
 }
 
 
+void MacroAssembler::PushPopQueue::PushQueued() {
+  if (queued_.empty()) return;
+
+  masm_->PrepareForPush(size_);
+
+  int count = queued_.size();
+  int index = 0;
+  while (index < count) {
+    // PushHelper can only handle registers with the same size and type, and it
+    // can handle only four at a time. Batch them up accordingly.
+    CPURegister batch[4] = {NoReg, NoReg, NoReg, NoReg};
+    int batch_index = 0;
+    do {
+      batch[batch_index++] = queued_[index++];
+    } while ((batch_index < 4) && (index < count) &&
+             batch[0].IsSameSizeAndType(queued_[index]));
+
+    masm_->PushHelper(batch_index, batch[0].SizeInBytes(),
+                      batch[0], batch[1], batch[2], batch[3]);
+  }
+
+  queued_.clear();
+}
+
+
+void MacroAssembler::PushPopQueue::PopQueued() {

[rmcilroy, 2014/02/17 12:31:06]

If I read this right, doing the following would not work:

PushPopQueue queue;
queue.Queue(x0);
queue.Queue(x1);

queue.PushQueued();

... Call function

queue.PopQueued();

Both because PushQueued() and PopQueued clear the queued_ field, and because the
arguments to Pop would need to be given in the reverse order as given to Push.
Would it be worth enabling this - it could save a load of boilerplate.  (Note: I
found it quite confusing that on arm you do Push(r0, r1), Pop(r0, r1), but on
a64 the Pop is reversed, e.g.,: Push(x0, x1), Pop(x1, x0) - could we unify this
to be the same as Arm?)

[jbramley, 2014/02/17 12:43:21]

That's correct. At the moment, PopQueued() isn't used (other than in the tests),
but I implemented it for completeness anyway.
We made a conscious decision to do things that way. On ARM, the registers are
pushed such that the lowest-indexed register gets the lowest address. Pop(r0,
r1) and Pop(r1, r0) do the same thing (though there may be a warning in V8). On
A64, that isn't the case, and we wanted the following sequences to be
equivalent:

Push(xa);
Push(xb);

Push(xa, xb);

That is, Push and Pop can be split into several calls, but the order of
operations always agrees with reading order. With this scheme, Pop has to work
in the opposite order. It would be easy to make PopQueued() reverse the list
automatically, but it would then operate differently from Pop(), as you noted.

Does that argument make sense? We spent some time thinking about it when we
started the port but that doesn't mean we can't change things.

+  if (queued_.empty()) return;
+
+  masm_->PrepareForPop(size_);
+
+  int count = queued_.size();
+  int index = 0;
+  while (index < count) {
+    // PopHelper can only handle registers with the same size and type, and it
+    // can handle only four at a time. Batch them up accordingly.
+    CPURegister batch[4] = {NoReg, NoReg, NoReg, NoReg};
+    int batch_index = 0;
+    do {
+      batch[batch_index++] = queued_[index++];
+    } while ((batch_index < 4) && (index < count) &&
+             batch[0].IsSameSizeAndType(queued_[index]));
+
+    masm_->PopHelper(batch_index, batch[0].SizeInBytes(),
+                     batch[0], batch[1], batch[2], batch[3]);
+  }
+
+  queued_.clear();
+}
+
+
 void MacroAssembler::PushCPURegList(CPURegList registers) {
   int size = registers.RegisterSizeInBytes();
 
   PrepareForPush(registers.Count(), size);
   // Push up to four registers at a time because if the current stack pointer is
   // csp and reg_size is 32, registers must be pushed in blocks of four in order
   // to maintain the 16-byte alignment for csp.
   while (!registers.IsEmpty()) {
     int count_before = registers.Count();
     const CPURegister& src0 = registers.PopHighestIndex();
@@ skipping 25 matching lines @@
 
   if (!csp.Is(StackPointer()) && emit_debug_code()) {
     // It is safe to leave csp where it is when unwinding the JavaScript stack,
     // but if we keep it matching StackPointer, the simulator can detect memory
     // accesses in the now-free part of the stack.
     Mov(csp, StackPointer());
   }
 }
 
 
-void MacroAssembler::PushMultipleTimes(int count, Register src) {
+void MacroAssembler::PushMultipleTimes(CPURegister src, int count) {
   int size = src.SizeInBytes();
 
   PrepareForPush(count, size);
 
   if (FLAG_optimize_for_size && count > 8) {
     Label loop;
     __ Mov(Tmp0(), count / 2);
     __ Bind(&loop);
     PushHelper(2, size, src, src, NoReg, NoReg);
     __ Subs(Tmp0(), Tmp0(), 1);
@@ skipping 14 matching lines @@
     count -= 2;
   }
   if (count == 1) {
     PushHelper(1, size, src, NoReg, NoReg, NoReg);
     count -= 1;
   }
   ASSERT(count == 0);
 }
 
 
+void MacroAssembler::PushMultipleTimes(CPURegister src, Register count) {
+  PrepareForPush(Operand(count, UXTW, WhichPowerOf2(src.SizeInBytes())));
+
+  Register temp = AppropriateTempFor(count);
+
+  if (FLAG_optimize_for_size) {
+    Label loop, done;
+
+    Subs(temp, count, 1);
+    B(mi, &done);
+
+    // Push all registers individually, to save code size.
+    Bind(&loop);
+    Subs(temp, temp, 1);
+    PushHelper(1, src.SizeInBytes(), src, NoReg, NoReg, NoReg);
+    B(pl, &loop);
+
+    Bind(&done);
+  } else {
+    Label loop, leftover2, leftover1, done;
+
+    Subs(temp, count, 4);
+    B(mi, &leftover2);
+
+    // Push groups of four first.
+    Bind(&loop);
+    Subs(temp, temp, 4);
+    PushHelper(4, src.SizeInBytes(), src, src, src, src);
+    B(pl, &loop);
+
+    // Push groups of two.
+    Bind(&leftover2);
+    Tbz(count, 1, &leftover1);
+    PushHelper(2, src.SizeInBytes(), src, src, NoReg, NoReg);
+
+    // Push the last one (if required).
+    Bind(&leftover1);
+    Tbz(count, 0, &done);
+    PushHelper(1, src.SizeInBytes(), src, NoReg, NoReg, NoReg);
+
+    Bind(&done);
+  }
+}
+
+
 void MacroAssembler::PushHelper(int count, int size,
                                 const CPURegister& src0,
                                 const CPURegister& src1,
                                 const CPURegister& src2,
                                 const CPURegister& src3) {
   // Ensure that we don't unintentially modify scratch or debug registers.
   InstructionAccurateScope scope(this);
 
   ASSERT(AreSameSizeAndType(src0, src1, src2, src3));
   ASSERT(size == src0.SizeInBytes());
@@ skipping 61 matching lines @@
       // for csp at all times.
       ldp(dst2, dst3, MemOperand(StackPointer(), 2 * size));
       ldp(dst0, dst1, MemOperand(StackPointer(), 4 * size, PostIndex));
       break;
     default:
       UNREACHABLE();
   }
 }
 
 
-void MacroAssembler::PrepareForPush(int count, int size) {
-  // TODO(jbramley): Use AssertStackConsistency here, if possible. See the
-  // AssertStackConsistency for details of why we can't at the moment.
-  if (csp.Is(StackPointer())) {
-    // If the current stack pointer is csp, then it must be aligned to 16 bytes
-    // on entry and the total size of the specified registers must also be a
-    // multiple of 16 bytes.
-    ASSERT((count * size) % 16 == 0);
-  } else {
-    // Even if the current stack pointer is not the system stack pointer (csp),
-    // the system stack pointer will still be modified in order to comply with
-    // ABI rules about accessing memory below the system stack pointer.
-    BumpSystemStackPointer(count * size);
-  }
-}
-
-
-void MacroAssembler::PrepareForPop(int count, int size) {
+void MacroAssembler::PrepareForPush(Operand total_size) {
   AssertStackConsistency();
   if (csp.Is(StackPointer())) {
     // If the current stack pointer is csp, then it must be aligned to 16 bytes
     // on entry and the total size of the specified registers must also be a
     // multiple of 16 bytes.
-    ASSERT((count * size) % 16 == 0);
+    if (total_size.IsImmediate()) {
+      ASSERT((total_size.immediate() % 16) == 0);
+    }
+
+    // Don't check access size for non-immediate sizes. It's difficult to do
+    // well, and it will be caught by hardware (or the simulator) anyway.
+  } else {
+    // Even if the current stack pointer is not the system stack pointer (csp),
+    // the system stack pointer will still be modified in order to comply with
+    // ABI rules about accessing memory below the system stack pointer.
+    BumpSystemStackPointer(total_size);
   }
 }
 
+
+void MacroAssembler::PrepareForPop(Operand total_size) {
+  AssertStackConsistency();
+  if (csp.Is(StackPointer())) {
+    // If the current stack pointer is csp, then it must be aligned to 16 bytes
+    // on entry and the total size of the specified registers must also be a
+    // multiple of 16 bytes.
+    if (total_size.IsImmediate()) {
+      ASSERT((total_size.immediate() % 16) == 0);
+    }
+
+    // Don't check access size for non-immediate sizes. It's difficult to do
+    // well, and it will be caught by hardware (or the simulator) anyway.
+  }
+}
+
 
 void MacroAssembler::Poke(const CPURegister& src, const Operand& offset) {
   if (offset.IsImmediate()) {
     ASSERT(offset.immediate() >= 0);
   } else if (emit_debug_code()) {
     Cmp(xzr, offset);
     Check(le, kStackAccessBelowStackPointer);
   }
 
   Str(src, MemOperand(StackPointer(), offset));
@@ skipping 72 matching lines @@
   ldp(x29, x30, tos);
 
   ldp(d8, d9, tos);
   ldp(d10, d11, tos);
   ldp(d12, d13, tos);
   ldp(d14, d15, tos);
 }
 
 
 void MacroAssembler::AssertStackConsistency() {
-  if (emit_debug_code() && !csp.Is(StackPointer())) {
+  if (emit_debug_code()) {
     if (csp.Is(StackPointer())) {
-      // TODO(jbramley): Check for csp alignment if it is the stack pointer.
-    } else {
-      // TODO(jbramley): Currently we cannot use this assertion in Push because
-      // some calling code assumes that the flags are preserved. For an example,
-      // look at Builtins::Generate_ArgumentsAdaptorTrampoline.
-      Cmp(csp, StackPointer());
-      Check(ls, kTheCurrentStackPointerIsBelowCsp);
+      // We can't check the alignment of csp without using a scratch register
+      // (or clobbering the flags), but the processor (or simulator) will abort
+      // if it is not properly aligned during a load.
+      ldr(xzr, MemOperand(csp, 0));
+    } else if (FLAG_enable_slow_asserts) {
+      Label ok;
+      // Check that csp <= StackPointer(), preserving all registers and NZCV.
+      sub(StackPointer(), csp, StackPointer());
+      cbz(StackPointer(), &ok);                 // Ok if csp == StackPointer().
+      tbnz(StackPointer(), kXSignBit, &ok);     // Ok if csp < StackPointer().
+
+      Abort(kTheCurrentStackPointerIsBelowCsp);
+
+      bind(&ok);
+      // Restore StackPointer().
+      sub(StackPointer(), csp, StackPointer());
     }
   }
 }
 
 
 void MacroAssembler::LoadRoot(Register destination,
                               Heap::RootListIndex index) {
   // TODO(jbramley): Most root values are constants, and can be synthesized
   // without a load. Refer to the ARM back end for details.
   Ldr(destination, MemOperand(root, index << kPointerSizeLog2));
@@ skipping 3412 matching lines @@
   if (FLAG_trap_on_abort) {
     Brk(0);
     return;
   }
 #endif
 
   Label msg_address;
   Adr(x0, &msg_address);
 
   if (use_real_aborts()) {
+    // Avoid infinite recursion; Push contains some assertions that use Abort.
+    NoUseRealAbortsScope no_real_aborts(this);
+
     // Split the message pointer into two SMI to avoid the GC
     // trying to scan the string.
     STATIC_ASSERT((kSmiShift == 32) && (kSmiTag == 0));
     SmiTag(x1, x0);
     Bic(x0, x0, kSmiShiftMask);
 
     Push(x0, x1);
 
     if (!has_frame_) {
       // We don't actually want to generate a pile of code for this, so just
@@ skipping 440 matching lines @@
   }
 }
 
 
 #undef __
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_A64