Bug 1523515 - Fix shuffling of two-valued return on ARM r=jandem a=lizzard
authorAndy Wingo <wingo@igalia.com>
Thu, 07 Feb 2019 13:50:20 +0000
changeset 515894 1d59770b712bbfb4b140c1ac8e179601871f8b94
parent 515893 c621e6736f7934b85e8d651adaefc7ed37fa3e63
child 515895 3e57fe219beb57862951db5c6ab6f109b23db212
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, lizzard
bugs1523515
milestone66.0
Bug 1523515 - Fix shuffling of two-valued return on ARM r=jandem a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D18822
js/src/jit/MacroAssembler.cpp
js/src/jit/MacroAssembler.h
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
--- a/js/src/jit/MacroAssembler.cpp
+++ b/js/src/jit/MacroAssembler.cpp
@@ -16,16 +16,17 @@
 #include "jit/AtomicOp.h"
 #include "jit/Bailouts.h"
 #include "jit/BaselineFrame.h"
 #include "jit/BaselineIC.h"
 #include "jit/BaselineJIT.h"
 #include "jit/JitOptions.h"
 #include "jit/Lowering.h"
 #include "jit/MIR.h"
+#include "jit/MoveEmitter.h"
 #include "js/Conversions.h"
 #include "js/Printf.h"
 #include "vm/TraceLogging.h"
 
 #include "gc/Nursery-inl.h"
 #include "jit/shared/Lowering-shared-inl.h"
 #include "jit/TemplateObject-inl.h"
 #include "vm/Interpreter-inl.h"
@@ -3178,16 +3179,39 @@ CodeOffset MacroAssembler::callWithABI(w
 // Exit frame footer.
 
 void MacroAssembler::linkExitFrame(Register cxreg, Register scratch) {
   loadPtr(Address(cxreg, JSContext::offsetOfActivation()), scratch);
   storeStackPtr(Address(scratch, JitActivation::offsetOfPackedExitFP()));
 }
 
 // ===============================================================
+// Simple value-shuffling helpers, to hide MoveResolver verbosity
+// in common cases.
+
+void MacroAssembler::moveRegPair(Register src0, Register src1, Register dst0,
+                                 Register dst1, MoveOp::Type type) {
+  MoveResolver& moves = moveResolver();
+  if (src0 != dst0) {
+    propagateOOM(moves.addMove(MoveOperand(src0), MoveOperand(dst0), type));
+  }
+  if (src1 != dst1) {
+    propagateOOM(moves.addMove(MoveOperand(src1), MoveOperand(dst1), type));
+  }
+  propagateOOM(moves.resolve());
+  if (oom()) {
+    return;
+  }
+
+  MoveEmitter emitter(*this);
+  emitter.emit(moves);
+  emitter.finish();
+}
+
+// ===============================================================
 // Branch functions
 
 void MacroAssembler::branchIfNotInterpretedConstructor(Register fun,
                                                        Register scratch,
                                                        Label* label) {
   // 16-bit loads are slow and unaligned 32-bit loads may be too so
   // perform an aligned 32-bit load and adjust the bitmask accordingly.
   static_assert(JSFunction::offsetOfNargs() % sizeof(uint32_t) == 0,
--- a/js/src/jit/MacroAssembler.h
+++ b/js/src/jit/MacroAssembler.h
@@ -451,16 +451,20 @@ class MacroAssembler : public MacroAssem
   // point of the callee.
   void callAndPushReturnAddress(Register reg) DEFINED_ON(x86_shared);
   void callAndPushReturnAddress(Label* label) DEFINED_ON(x86_shared);
 
   // These do not adjust framePushed().
   void pushReturnAddress() DEFINED_ON(mips_shared, arm, arm64);
   void popReturnAddress() DEFINED_ON(mips_shared, arm, arm64);
 
+  // Useful for dealing with two-valued returns.
+  void moveRegPair(Register src0, Register src1, Register dst0, Register dst1,
+                   MoveOp::Type type = MoveOp::GENERAL);
+
  public:
   // ===============================================================
   // Patchable near/far jumps.
 
   // "Far jumps" provide the ability to jump to any uint32_t offset from any
   // other uint32_t offset without using a constant pool (thus returning a
   // simple CodeOffset instead of a CodeOffsetJump).
   CodeOffset farJumpWithPatch() PER_SHARED_ARCH;
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -5679,18 +5679,17 @@ void MacroAssembler::flexibleDivMod32(Re
       ScratchRegisterScope scratch(*this);
       setupUnalignedABICall(scratch);
     }
     passABIArg(lhsOutput);
     passABIArg(rhs);
     callWithABI(isUnsigned ? JS_FUNC_TO_DATA_PTR(void*, __aeabi_uidivmod)
                            : JS_FUNC_TO_DATA_PTR(void*, __aeabi_idivmod),
                 MoveOp::GENERAL, CheckUnsafeCallWithABI::DontCheckOther);
-    mov(ReturnRegVal1, remOutput);
-    mov(ReturnRegVal0, lhsOutput);
+    moveRegPair(ReturnRegVal0, ReturnRegVal1, lhsOutput, remOutput);
 
     LiveRegisterSet ignore;
     ignore.add(remOutput);
     ignore.add(lhsOutput);
     PopRegsInMaskIgnore(volatileLiveRegs, ignore);
   }
 }
 
--- a/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
+++ b/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
@@ -249,45 +249,16 @@ void MacroAssemblerX86Shared::minMaxFloa
 //{{{ check_macroassembler_style
 // ===============================================================
 // MacroAssembler high-level usage.
 
 void MacroAssembler::flush() {}
 
 void MacroAssembler::comment(const char* msg) { masm.comment(msg); }
 
-class MOZ_RAII ScopedMoveResolution {
-  MacroAssembler& masm_;
-  MoveResolver& resolver_;
-
- public:
-  explicit ScopedMoveResolution(MacroAssembler& masm)
-      : masm_(masm), resolver_(masm.moveResolver()) {}
-
-  void addMove(Register src, Register dest) {
-    if (src != dest) {
-      masm_.propagateOOM(resolver_.addMove(MoveOperand(src), MoveOperand(dest),
-                                           MoveOp::GENERAL));
-    }
-  }
-
-  ~ScopedMoveResolution() {
-    masm_.propagateOOM(resolver_.resolve());
-    if (masm_.oom()) {
-      return;
-    }
-
-    resolver_.sortMemoryToMemoryMoves();
-
-    MoveEmitter emitter(masm_);
-    emitter.emit(resolver_);
-    emitter.finish();
-  }
-};
-
 // This operation really consists of five phases, in order to enforce the
 // restriction that on x86_shared, srcDest must be eax and edx will be
 // clobbered.
 //
 //     Input: { rhs, lhsOutput }
 //
 //  [PUSH] Preserve registers
 //  [MOVE] Generate moves to specific registers
@@ -319,40 +290,32 @@ void MacroAssembler::flexibleDivMod32(Re
   preserve.add(eax);
   preserve.add(regForRhs);
 
   preserve.takeUnchecked(lhsOutput);
   preserve.takeUnchecked(remOutput);
 
   PushRegsInMask(preserve);
 
-  // Marshal Registers For operation
-  {
-    ScopedMoveResolution resolution(*this);
-    resolution.addMove(rhs, regForRhs);
-    resolution.addMove(lhsOutput, eax);
-  }
+  // Shuffle input into place.
+  moveRegPair(lhsOutput, rhs, eax, regForRhs);
   if (oom()) {
     return;
   }
 
   // Sign extend eax into edx to make (edx:eax): idiv/udiv are 64-bit.
   if (isUnsigned) {
     mov(ImmWord(0), edx);
     udiv(regForRhs);
   } else {
     cdq();
     idiv(regForRhs);
   }
 
-  {
-    ScopedMoveResolution resolution(*this);
-    resolution.addMove(eax, lhsOutput);
-    resolution.addMove(edx, remOutput);
-  }
+  moveRegPair(eax, edx, lhsOutput, remOutput);
   if (oom()) {
     return;
   }
 
   PopRegsInMask(preserve);
 }
 
 void MacroAssembler::flexibleQuotient32(