Bug 1619343 - Part 1: Move zero arguments jump out of emitAllocateSpaceForApply. r=jandem
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 05 Mar 2020 16:01:44 +0000
changeset 517087 3e88ffbc21688ba41e48ec585f9e9b402a8b9668
parent 517086 ead821e4297c370ad13459718e4b1cf1ae339972
child 517088 74c24cb3ac9a99e776ed265767342c8c504b1e1d
push id109226
push usercsabou@mozilla.com
push dateThu, 05 Mar 2020 16:22:43 +0000
treeherderautoland@0bbb01c42e2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1619343
milestone75.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 1619343 - Part 1: Move zero arguments jump out of emitAllocateSpaceForApply. r=jandem Move the branch instruction out of `emitAllocateSpaceForApply()` to make it easier to follow the control flow. Drive-by fix: - Reformat a comment which was broken into two lines by clang-format. - Use the correct lir-class when accessing `ThisIndex` for `LApplyArrayGeneric`. Differential Revision: https://phabricator.services.mozilla.com/D64978
js/src/jit/CodeGenerator.cpp
js/src/jit/CodeGenerator.h
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -5284,18 +5284,17 @@ void CodeGenerator::emitCallInvokeFuncti
   callVM<Fn, jit::InvokeFunction>(apply, &extraStackSize);
 
   masm.Pop(extraStackSize);
 }
 
 // Do not bailout after the execution of this function since the stack no longer
 // correspond to what is expected by the snapshots.
 void CodeGenerator::emitAllocateSpaceForApply(Register argcreg,
-                                              Register extraStackSpace,
-                                              Label* end) {
+                                              Register extraStackSpace) {
   // Initialize the loop counter AND Compute the stack usage (if == 0)
   masm.movePtr(argcreg, extraStackSpace);
 
   // Align the JitFrameLayout on the JitStackAlignment.
   if (JitStackValueAlignment > 1) {
     MOZ_ASSERT(frameSize() % JitStackAlignment == 0,
                "Stack padding assumes that the frameSize is correct");
     MOZ_ASSERT(JitStackValueAlignment == 2);
@@ -5320,19 +5319,16 @@ void CodeGenerator::emitAllocateSpaceFor
     Label noPaddingNeeded;
     // if the number of arguments is odd, then we do not need any padding.
     masm.branchTestPtr(Assembler::NonZero, argcreg, Imm32(1), &noPaddingNeeded);
     BaseValueIndex dstPtr(masm.getStackPointer(), argcreg);
     masm.storeValue(MagicValue(JS_ARG_POISON), dstPtr);
     masm.bind(&noPaddingNeeded);
   }
 #endif
-
-  // Skip the copy of arguments if there are none.
-  masm.branchTestPtr(Assembler::Zero, argcreg, argcreg, end);
 }
 
 // Destroys argvIndex and copyreg.
 void CodeGenerator::emitCopyValuesForApply(Register argvSrcBase,
                                            Register argvIndex, Register copyreg,
                                            size_t argvSrcOffset,
                                            size_t argvDstOffset) {
   Label loop;
@@ -5367,23 +5363,29 @@ void CodeGenerator::emitPopArguments(Reg
 
 void CodeGenerator::emitPushArguments(LApplyArgsGeneric* apply,
                                       Register extraStackSpace) {
   // Holds the function nargs. Initially the number of args to the caller.
   Register argcreg = ToRegister(apply->getArgc());
   Register copyreg = ToRegister(apply->getTempObject());
 
   Label end;
-  emitAllocateSpaceForApply(argcreg, extraStackSpace, &end);
-
+  emitAllocateSpaceForApply(argcreg, extraStackSpace);
+
+  // Skip the copy of arguments if there are none.
+  masm.branchTestPtr(Assembler::Zero, argcreg, argcreg, &end);
+
+  // clang-format off
+  //
   // We are making a copy of the arguments which are above the JitFrameLayout
   // of the current Ion frame.
   //
-  // [arg1] [arg0] <- src [this] [JitFrameLayout] [.. frameSize ..] [pad] [arg1]
-  // [arg0] <- dst
+  // [arg1] [arg0] <- src [this] [JitFrameLayout] [.. frameSize ..] [pad] [arg1] [arg0] <- dst
+  //
+  // clang-format on
 
   // Compute the source and destination offsets into the stack.
   size_t argvSrcOffset = frameSize() + JitFrameLayout::offsetOfActualArgs();
   size_t argvDstOffset = 0;
 
   // Save the extra stack space, and re-use the register as a base.
   masm.push(extraStackSpace);
   Register argvSrcBase = extraStackSpace;
@@ -5428,17 +5430,20 @@ void CodeGenerator::emitPushArguments(LA
   //  - the array length equals its initialized length
 
   // The array length is our argc for the purposes of allocating space.
   Address length(ToRegister(apply->getElements()),
                  ObjectElements::offsetOfLength());
   masm.load32(length, tmpArgc);
 
   // Allocate space for the values.
-  emitAllocateSpaceForApply(tmpArgc, extraStackSpace, &noCopy);
+  emitAllocateSpaceForApply(tmpArgc, extraStackSpace);
+
+  // Skip the copy of arguments if there are none.
+  masm.branchTestPtr(Assembler::Zero, tmpArgc, tmpArgc, &noCopy);
 
   // Copy the values.  This code is skipped entirely if there are
   // no values.
   size_t argvDstOffset = 0;
 
   Register argvSrcBase = elementsAndArgc;  // Elements value
 
   masm.push(extraStackSpace);
@@ -5462,17 +5467,17 @@ void CodeGenerator::emitPushArguments(LA
   masm.movePtr(ImmPtr(0), elementsAndArgc);
 
   // Join with all arguments copied and the extra stack usage computed.
   // Note, "elements" has become "argc".
   masm.bind(&epilogue);
 
   // Push |this|.
   masm.addPtr(Imm32(sizeof(Value)), extraStackSpace);
-  masm.pushValue(ToValue(apply, LApplyArgsGeneric::ThisIndex));
+  masm.pushValue(ToValue(apply, LApplyArrayGeneric::ThisIndex));
 }
 
 template <typename T>
 void CodeGenerator::emitApplyGeneric(T* apply) {
   // Holds the function object.
   Register calleereg = ToRegister(apply->getFunction());
 
   // Temporary register for modifying the function object.
--- a/js/src/jit/CodeGenerator.h
+++ b/js/src/jit/CodeGenerator.h
@@ -159,18 +159,17 @@ class CodeGenerator final : public CodeG
 
   void emitCallInvokeFunction(LInstruction* call, Register callereg,
                               bool isConstructing, bool ignoresReturnValue,
                               uint32_t argc, uint32_t unusedStack);
   template <typename T>
   void emitApplyGeneric(T* apply);
   template <typename T>
   void emitCallInvokeFunction(T* apply, Register extraStackSize);
-  void emitAllocateSpaceForApply(Register argcreg, Register extraStackSpace,
-                                 Label* end);
+  void emitAllocateSpaceForApply(Register argcreg, Register extraStackSpace);
   void emitCopyValuesForApply(Register argvSrcBase, Register argvIndex,
                               Register copyreg, size_t argvSrcOffset,
                               size_t argvDstOffset);
   void emitPopArguments(Register extraStackSize);
   void emitPushArguments(LApplyArgsGeneric* apply, Register extraStackSpace);
   void emitPushArguments(LApplyArrayGeneric* apply, Register extraStackSpace);
 
   void visitNewArrayCallVM(LNewArray* lir);