Bug 1299147 - Split double MoveOperands that conflict with floats. r=jandem, a=gchang
authorSean Stangl <sstangl@mozilla.com>
Thu, 04 May 2017 15:39:52 -0700
changeset 396305 adb9ebd81ba2a74931d9e932bfcc1d1f6aca9e17
parent 396304 7067d6a7dc74d854498ff5f45a45b74431e21160
child 396306 57782b6323abae342864c4a29bb802decb7914fe
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, gchang
bugs1299147
milestone54.0
Bug 1299147 - Split double MoveOperands that conflict with floats. r=jandem, a=gchang
js/src/jit/MoveResolver.cpp
js/src/jit/MoveResolver.h
js/src/jsapi-tests/testJitMoveEmitterCycles.cpp
--- a/js/src/jit/MoveResolver.cpp
+++ b/js/src/jit/MoveResolver.cpp
@@ -101,22 +101,121 @@ MoveResolver::findCycledMove(PendingMove
             (*iter)++;
             return other;
         }
     }
     // No blocking moves found.
     return nullptr;
 }
 
+#ifdef JS_CODEGEN_ARM
+static inline bool
+MoveIsDouble(const MoveOperand& move)
+{
+    if (!move.isFloatReg())
+        return false;
+    return move.floatReg().isDouble();
+}
+#endif
+
+#ifdef JS_CODEGEN_ARM
+static inline bool
+MoveIsSingle(const MoveOperand& move)
+{
+    if (!move.isFloatReg())
+        return false;
+    return move.floatReg().isSingle();
+}
+#endif
+
+#ifdef JS_CODEGEN_ARM
+bool
+MoveResolver::isDoubleAliasedAsSingle(const MoveOperand& move)
+{
+    if (!MoveIsDouble(move))
+        return false;
+
+    for (auto iter = pending_.begin(); iter != pending_.end(); ++iter) {
+        PendingMove* other = *iter;
+        if (other->from().aliases(move) && MoveIsSingle(other->from()))
+            return true;
+        if (other->to().aliases(move) && MoveIsSingle(other->to()))
+            return true;
+    }
+    return false;
+}
+#endif
+
+#ifdef JS_CODEGEN_ARM
+static MoveOperand
+SplitIntoLowerHalf(const MoveOperand& move)
+{
+    if (MoveIsDouble(move)) {
+        FloatRegister lowerSingle = move.floatReg().asSingle();
+        return MoveOperand(lowerSingle);
+    }
+
+    MOZ_ASSERT(move.isMemoryOrEffectiveAddress());
+    return move;
+}
+#endif
+
+#ifdef JS_CODEGEN_ARM
+static MoveOperand
+SplitIntoUpperHalf(const MoveOperand& move)
+{
+    if (MoveIsDouble(move)) {
+        FloatRegister lowerSingle = move.floatReg().asSingle();
+        FloatRegister upperSingle = VFPRegister(lowerSingle.code() + 1, VFPRegister::Single);
+        return MoveOperand(upperSingle);
+    }
+
+    MOZ_ASSERT(move.isMemoryOrEffectiveAddress());
+    return MoveOperand(move.base(), move.disp() + sizeof(float));
+}
+#endif
+
 bool
 MoveResolver::resolve()
 {
     resetState();
     orderedMoves_.clear();
 
+#ifdef JS_CODEGEN_ARM
+    // Some of ARM's double registers alias two of its single registers,
+    // but the algorithm below assumes that every register can participate
+    // in at most one cycle. To satisfy the algorithm, any double registers
+    // that may conflict are split into their single-register halves.
+    //
+    // This logic is only applicable because ARM only uses registers d0-d15,
+    // all of which alias s0-s31. Double registers d16-d31 are unused.
+    // Therefore there is never a double move that cannot be split.
+    // If this changes in the future, the algorithm will have to be fixed.
+    for (auto iter = pending_.begin(); iter != pending_.end(); ++iter) {
+        PendingMove* pm = *iter;
+
+        if (isDoubleAliasedAsSingle(pm->from()) || isDoubleAliasedAsSingle(pm->to())) {
+            PendingMove* lower = movePool_.allocate();
+            if (!lower)
+                return false;
+
+            // Insert the new node before the current position to not affect iteration.
+            MoveOperand fromLower = SplitIntoLowerHalf(pm->from());
+            MoveOperand toLower = SplitIntoLowerHalf(pm->to());
+            new (lower) PendingMove(fromLower, toLower, MoveOp::FLOAT32);
+            pending_.insertBefore(pm, lower);
+
+            // Overwrite pm in place for the upper move. Iteration proceeds as normal.
+            MoveOperand fromUpper = SplitIntoUpperHalf(pm->from());
+            MoveOperand toUpper = SplitIntoUpperHalf(pm->to());
+            pm->overwrite(fromUpper, toUpper, MoveOp::FLOAT32);
+        }
+    }
+#endif
+
     InlineList<PendingMove> stack;
 
     // This is a depth-first-search without recursion, which tries to find
     // cycles in a list of moves.
     //
     // Algorithm.
     //
     // S = Traversal stack.
--- a/js/src/jit/MoveResolver.h
+++ b/js/src/jit/MoveResolver.h
@@ -247,16 +247,23 @@ class MoveOp
         return endCycleType_;
     }
     bool aliases(const MoveOperand& op) const {
         return from().aliases(op) || to().aliases(op);
     }
     bool aliases(const MoveOp& other) const {
         return aliases(other.from()) || aliases(other.to());
     }
+#ifdef JS_CODEGEN_ARM
+    void overwrite(MoveOperand& from, MoveOperand& to, Type type) {
+        from_ = from;
+        to_ = to;
+        type_ = type;
+    }
+#endif
 };
 
 class MoveResolver
 {
   private:
     struct PendingMove
       : public MoveOp,
         public TempObject,
@@ -294,16 +301,20 @@ class MoveResolver
     PendingMove* findBlockingMove(const PendingMove* last);
     PendingMove* findCycledMove(PendingMoveIterator* stack, PendingMoveIterator end, const PendingMove* first);
     MOZ_MUST_USE bool addOrderedMove(const MoveOp& move);
     void reorderMove(size_t from, size_t to);
 
     // Internal reset function. Does not clear lists.
     void resetState();
 
+#ifdef JS_CODEGEN_ARM
+    bool isDoubleAliasedAsSingle(const MoveOperand& move);
+#endif
+
   public:
     MoveResolver();
 
     // Resolves a move group into two lists of ordered moves. These moves must
     // be executed in the order provided. Some moves may indicate that they
     // participate in a cycle. For every cycle there are two such moves, and it
     // is guaranteed that cycles do not nest inside each other in the list.
     //
--- a/js/src/jsapi-tests/testJitMoveEmitterCycles.cpp
+++ b/js/src/jsapi-tests/testJitMoveEmitterCycles.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #if defined(JS_SIMULATOR_ARM)
+
 #include "jit/arm/Assembler-arm.h"
 #include "jit/arm/MoveEmitter-arm.h"
 #include "jit/arm/Simulator-arm.h"
 #include "jit/Linker.h"
 #include "jit/MacroAssembler.h"
 #include "jit/MoveResolver.h"
 
 #include "jsapi-tests/tests.h"
@@ -523,10 +524,101 @@ BEGIN_TEST(testJitMoveEmitterCycles_auto
     CHECK(int(f) == 29);
     sim->get_float_from_s_register(15, &f);
     CHECK(int(f) == 29);
     sim->get_float_from_s_register(23, &f);
     CHECK(int(f) == 16);
     return true;
 }
 END_TEST(testJitMoveEmitterCycles_autogen3)
+BEGIN_TEST(testJitMoveEmitterCycles_bug1299147_1)
+{
+    using namespace js;
+    using namespace js::jit;
+    LifoAlloc lifo(LIFO_ALLOC_PRIMARY_CHUNK_SIZE);
+    TempAllocator alloc(&lifo);
+    JitContext jc(cx, &alloc);
+    cx->runtime()->getJitRuntime(cx);
+    MacroAssembler masm;
+    MoveEmitter mover(masm);
+    MoveResolver mr;
+    mr.setAllocator(alloc);
+    Simulator* sim = Simulator::Current();
+    // S2 -> S0
+    // S2 -> S6
+    // S3 -> S1
+    // S3 -> S7
+    // D0 -> D1
+    // D0 -> D2
+    TRY(mr.addMove(MoveOperand(s2), MoveOperand(s0), MoveOp::FLOAT32));
+    TRY(mr.addMove(MoveOperand(s2), MoveOperand(s6), MoveOp::FLOAT32));
+    sim->set_s_register_from_float(2, 2);
+    TRY(mr.addMove(MoveOperand(s3), MoveOperand(s1), MoveOp::FLOAT32));
+    TRY(mr.addMove(MoveOperand(s3), MoveOperand(s7), MoveOp::FLOAT32));
+    sim->set_s_register_from_float(3, 4);
+    TRY(mr.addMove(MoveOperand(d0), MoveOperand(d1), MoveOp::FLOAT32));
+    TRY(mr.addMove(MoveOperand(d0), MoveOperand(d2), MoveOp::FLOAT32));
+    sim->set_d_register_from_double(0, 1);
+    // don't explode!
+    TRY(mr.resolve());
+    mover.emit(mr);
+    mover.finish();
+    masm.abiret();
+    JitCode* code = linkAndAllocate(cx, &masm);
+    sim->call(code->raw(), 1, 1);
+    float f;
+    double d;
+    sim->get_double_from_d_register(1, &d);
+    CHECK(d == 1);
+    sim->get_double_from_d_register(2, &d);
+    CHECK(d == 1);
+    sim->get_float_from_s_register(0, &f);
+    CHECK(int(f) == 2);
+    sim->get_float_from_s_register(6, &f);
+    CHECK(int(f) == 2);
+    sim->get_float_from_s_register(1, &f);
+    CHECK(int(f) == 4);
+    sim->get_float_from_s_register(7, &f);
+    CHECK(int(f) == 4);
+    return true;
+}
+END_TEST(testJitMoveEmitterCycles_bug1299147_1)
+BEGIN_TEST(testJitMoveEmitterCycles_bug1299147)
+{
+    using namespace js;
+    using namespace js::jit;
+    LifoAlloc lifo(LIFO_ALLOC_PRIMARY_CHUNK_SIZE);
+    TempAllocator alloc(&lifo);
+    JitContext jc(cx, &alloc);
+    cx->runtime()->getJitRuntime(cx);
+    MacroAssembler masm;
+    MoveEmitter mover(masm);
+    MoveResolver mr;
+    mr.setAllocator(alloc);
+    Simulator* sim = Simulator::Current();
+    // S2 -> S5
+    // S2 -> S6
+    // D0 -> D1
+    TRY(mr.addMove(MoveOperand(s2), MoveOperand(s5), MoveOp::FLOAT32));
+    TRY(mr.addMove(MoveOperand(s2), MoveOperand(s6), MoveOp::FLOAT32));
+    sim->set_s_register_from_float(2, 2);
+    TRY(mr.addMove(MoveOperand(d0), MoveOperand(d1), MoveOp::FLOAT32));
+    sim->set_d_register_from_double(0, 1);
+    // don't explode!
+    TRY(mr.resolve());
+    mover.emit(mr);
+    mover.finish();
+    masm.abiret();
+    JitCode* code = linkAndAllocate(cx, &masm);
+    sim->call(code->raw(), 1, 1);
+    float f;
+    double d;
+    sim->get_double_from_d_register(1, &d);
+    CHECK(d == 1);
+    sim->get_float_from_s_register(5, &f);
+    CHECK(int(f) == 2);
+    sim->get_float_from_s_register(6, &f);
+    CHECK(int(f) == 2);
+    return true;
+}
+END_TEST(testJitMoveEmitterCycles_bug1299147)
 
-#endif
+#endif // JS_SIMULATOR_ARM