Bug 1375074 - Save and restore non-volatile x28 on ARM64 for generated unboxed object constructor. r=sstangl
authorLars T Hansen <lhansen@mozilla.com>
Wed, 28 Feb 2018 13:57:52 +0100
changeset 463960 800abe66894d6b07b24bccecbf6a65e2261076f6
parent 463959 223c97459e96183eb616aed39147207bdb953ba8
child 463961 1aa685a4d83836790713bc1a03d4217d6d5d44f1
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstangl
bugs1375074
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1375074 - Save and restore non-volatile x28 on ARM64 for generated unboxed object constructor. r=sstangl
js/src/jit-test/tests/bug1375074.js
js/src/vm/UnboxedObject.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/bug1375074.js
@@ -0,0 +1,18 @@
+// This forces the VM to start creating unboxed objects and thus stresses a
+// particular path into generated code for a specialized unboxed object
+// constructor.
+
+var K = 2000;			// 2000 should be plenty
+var s = "[";
+var i;
+for ( i=0; i < K-1; i++ )
+    s = s + `{"i":${i}},`;
+s += `{"i":${i}}]`;
+var v = JSON.parse(s);
+
+assertEq(v.length == K, true);
+
+for ( i=0; i < K; i++) {
+    assertEq(v[i] instanceof Object, true);
+    assertEq(v[i].i, i);
+}
--- a/js/src/vm/UnboxedObject.cpp
+++ b/js/src/vm/UnboxedObject.cpp
@@ -90,17 +90,25 @@ UnboxedLayout::makeConstructorCode(JSCon
     masm.loadPtr(Address(masm.getStackPointer(), sizeof(void*)), propertiesReg);
     masm.loadPtr(Address(masm.getStackPointer(), 2 * sizeof(void*)), newKindReg);
 #else
     propertiesReg = IntArgReg0;
     newKindReg = IntArgReg1;
 #endif
 
 #ifdef JS_CODEGEN_ARM64
-    // ARM64 communicates stack address via sp, but uses a pseudo-sp for addressing.
+    // ARM64 communicates stack address via sp, but uses a pseudo-sp (PSP) for
+    // addressing.  The register we use for PSP may however also be used by
+    // calling code, and it is nonvolatile, so save it.  Do this as a special
+    // case first because the generic save/restore code needs the PSP to be
+    // initialized already.
+    MOZ_ASSERT(PseudoStackPointer64.Is(masm.GetStackPointer64()));
+    masm.Str(PseudoStackPointer64, vixl::MemOperand(sp, -16, vixl::PreIndex));
+
+    // Initialize the PSP from the SP.
     masm.initStackPtr();
 #endif
 
     MOZ_ASSERT(propertiesReg.volatile_());
     MOZ_ASSERT(newKindReg.volatile_());
 
     AllocatableGeneralRegisterSet regs(GeneralRegisterSet::All());
     regs.take(propertiesReg);
@@ -228,17 +236,32 @@ UnboxedLayout::makeConstructorCode(JSCon
     if (object != ReturnReg)
         masm.movePtr(object, ReturnReg);
 
     // Restore non-volatile registers which were saved on entry.
     if (ScratchDoubleReg.volatile_())
         masm.pop(ScratchDoubleReg);
     masm.PopRegsInMask(savedNonVolatileRegisters);
 
+#ifdef JS_CODEGEN_ARM64
+    // Now restore the value that was in the PSP register on entry, and return.
+
+    // Obtain the correct SP from the PSP.
+    masm.Mov(sp, PseudoStackPointer64);
+
+    // Restore the saved value of the PSP register, this value is whatever the
+    // caller had saved in it, not any actual SP value, and it must not be
+    // overwritten subsequently.
+    masm.Ldr(PseudoStackPointer64, vixl::MemOperand(sp, 16, vixl::PostIndex));
+
+    // Perform a plain Ret(), as abiret() will move SP <- PSP and that is wrong.
+    masm.Ret(vixl::lr);
+#else
     masm.abiret();
+#endif
 
     masm.bind(&failureStoreOther);
 
     // There was a failure while storing a value which cannot be stored at all
     // in the unboxed object. Initialize the object so it is safe for GC and
     // return null.
     masm.initUnboxedObjectContents(object, templateObject);