[turbofan] Refactor AssembleMove and AssembleSwap

The way the code generator's AssembleMove and AssembleSwap methods are written
makes it easy to forget which sort of move is being implemented when looking at
a sequence of instructions. This patch is an attempt to address this by
rewriting those methods using switch/case instead of a string of if/else.

To do this, introduce new utility functions to detect what type of move to
perform given a pair of InstructionOperands.

Bug: 
Change-Id: I32b146c86409e595b7b59a66bf43220899024fdd
Reviewed-on: https://chromium-review.googlesource.com/749201
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50966}

src/arm/macro-assembler-arm.cc

@@ -241,22 +241,6 @@ void TurboAssembler::Ret(int drop, Condition cond) {
   Ret(cond);
 }
 
-
-void MacroAssembler::Swap(Register reg1,
-                          Register reg2,
-                          Register scratch,
-                          Condition cond) {
-  if (scratch == no_reg) {
-    eor(reg1, reg1, Operand(reg2), LeaveCC, cond);
-    eor(reg2, reg2, Operand(reg1), LeaveCC, cond);
-    eor(reg1, reg1, Operand(reg2), LeaveCC, cond);
-  } else {
-    mov(scratch, reg1, LeaveCC, cond);
-    mov(reg1, reg2, LeaveCC, cond);
-    mov(reg2, scratch, LeaveCC, cond);
-  }
-}
-
 void TurboAssembler::Call(Label* target) { bl(target); }
 
 void TurboAssembler::Push(Handle<HeapObject> handle) {
@@ -305,9 +289,17 @@ void TurboAssembler::Move(QwNeonRegister dst, QwNeonRegister src) {
   }
 }
 
-void TurboAssembler::Swap(DwVfpRegister srcdst0, DwVfpRegister srcdst1) {
-  if (srcdst0 == srcdst1) return;  // Swapping aliased registers emits nothing.
+void TurboAssembler::Swap(Register srcdst0, Register srcdst1) {
+  DCHECK(srcdst0 != srcdst1);
+  UseScratchRegisterScope temps(this);
+  Register scratch = temps.Acquire();
+  mov(scratch, srcdst0);
+  mov(srcdst0, srcdst1);
+  mov(srcdst1, scratch);
+}
 
+void TurboAssembler::Swap(DwVfpRegister srcdst0, DwVfpRegister srcdst1) {
+  DCHECK(srcdst0 != srcdst1);
   DCHECK(VfpRegisterIsAvailable(srcdst0));
   DCHECK(VfpRegisterIsAvailable(srcdst1));
 
@@ -323,9 +315,8 @@ void TurboAssembler::Swap(DwVfpRegister srcdst0, DwVfpRegister srcdst1) {
 }
 
 void TurboAssembler::Swap(QwNeonRegister srcdst0, QwNeonRegister srcdst1) {
-  if (srcdst0 != srcdst1) {
-    vswp(srcdst0, srcdst1);
-  }
+  DCHECK(srcdst0 != srcdst1);
+  vswp(srcdst0, srcdst1);
 }
 
 void MacroAssembler::Mls(Register dst, Register src1, Register src2,

src/arm/macro-assembler-arm.h

@@ -482,7 +482,8 @@ class TurboAssembler : public Assembler {
   void VmovExtended(int dst_code, const MemOperand& src);
   void VmovExtended(const MemOperand& dst, int src_code);
 
-  // Register swap.
+  // Register swap. Note that the register operands should be distinct.
+  void Swap(Register srcdst0, Register srcdst1);
   void Swap(DwVfpRegister srcdst0, DwVfpRegister srcdst1);
   void Swap(QwNeonRegister srcdst0, QwNeonRegister srcdst1);
 
@@ -580,11 +581,6 @@ class MacroAssembler : public TurboAssembler {
   MacroAssembler(Isolate* isolate, void* buffer, int size,
                  CodeObjectRequired create_code_object);
 
-  // Swap two registers.  If the scratch register is omitted then a slightly
-  // less efficient form using xor instead of mov is emitted.
-  void Swap(Register reg1, Register reg2, Register scratch = no_reg,
-            Condition cond = al);
-
   void Mls(Register dst, Register src1, Register src2, Register srcA,
            Condition cond = al);
   void And(Register dst, Register src1, const Operand& src2,

src/arm64/macro-assembler-arm64.cc

@@ -1571,6 +1571,34 @@ void TurboAssembler::Move(Register dst, Register src) { Mov(dst, src); }
 void TurboAssembler::Move(Register dst, Handle<HeapObject> x) { Mov(dst, x); }
 void TurboAssembler::Move(Register dst, Smi* src) { Mov(dst, src); }
 
+void TurboAssembler::Swap(Register lhs, Register rhs) {
+  DCHECK(lhs.IsSameSizeAndType(rhs));
+  DCHECK(!lhs.Is(rhs));
+  UseScratchRegisterScope temps(this);
+  Register temp = temps.AcquireX();
+  Mov(temp, rhs);
+  Mov(rhs, lhs);
+  Mov(lhs, temp);
+}
+
+void TurboAssembler::Swap(VRegister lhs, VRegister rhs) {
+  DCHECK(lhs.IsSameSizeAndType(rhs));
+  DCHECK(!lhs.Is(rhs));
+  UseScratchRegisterScope temps(this);
+  VRegister temp = VRegister::no_reg();
+  if (lhs.IsS()) {
+    temp = temps.AcquireS();
+  } else if (lhs.IsD()) {
+    temp = temps.AcquireD();
+  } else {
+    DCHECK(lhs.IsQ());
+    temp = temps.AcquireQ();
+  }
+  Mov(temp, rhs);
+  Mov(rhs, lhs);
+  Mov(lhs, temp);
+}
+
 void TurboAssembler::AssertSmi(Register object, AbortReason reason) {
   if (emit_debug_code()) {
     STATIC_ASSERT(kSmiTag == 0);

src/arm64/macro-assembler-arm64.h

@@ -255,6 +255,10 @@ class TurboAssembler : public Assembler {
   void Move(Register dst, Handle<HeapObject> x);
   void Move(Register dst, Smi* src);
 
+  // Register swap. Note that the register operands should be distinct.
+  void Swap(Register lhs, Register rhs);
+  void Swap(VRegister lhs, VRegister rhs);
+
 // NEON by element instructions.
 #define NEON_BYELEMENT_MACRO_LIST(V) \
   V(fmla, Fmla)                      \
@@ -2104,6 +2108,7 @@ class UseScratchRegisterScope {
   Register AcquireX() { return AcquireNextAvailable(available_).X(); }
   VRegister AcquireS() { return AcquireNextAvailable(availablefp_).S(); }
   VRegister AcquireD() { return AcquireNextAvailable(availablefp_).D(); }
+  VRegister AcquireQ() { return AcquireNextAvailable(availablefp_).Q(); }
   VRegister AcquireV(VectorFormat format) {
     return VRegister::Create(AcquireNextAvailable(availablefp_).code(), format);
   }

src/compiler/code-generator.cc

@@ -488,6 +488,54 @@ void CodeGenerator::GetPushCompatibleMoves(Instruction* instr,
   pushes->resize(push_count);
 }
 
+CodeGenerator::MoveType::Type CodeGenerator::MoveType::InferMove(
+    InstructionOperand* source, InstructionOperand* destination) {
+  if (source->IsConstant()) {
+    if (destination->IsAnyRegister()) {
+      return MoveType::kConstantToRegister;
+    } else {
+      DCHECK(destination->IsAnyStackSlot());
+      return MoveType::kConstantToStack;
+    }
+  }
+  DCHECK(LocationOperand::cast(source)->IsCompatible(
+      LocationOperand::cast(destination)));
+  if (source->IsAnyRegister()) {
+    if (destination->IsAnyRegister()) {
+      return MoveType::kRegisterToRegister;
+    } else {
+      DCHECK(destination->IsAnyStackSlot());
+      return MoveType::kRegisterToStack;
+    }
+  } else {
+    DCHECK(source->IsAnyStackSlot());
+    if (destination->IsAnyRegister()) {
+      return MoveType::kStackToRegister;
+    } else {
+      DCHECK(destination->IsAnyStackSlot());
+      return MoveType::kStackToStack;
+    }
+  }
+}
+
+CodeGenerator::MoveType::Type CodeGenerator::MoveType::InferSwap(
+    InstructionOperand* source, InstructionOperand* destination) {
+  DCHECK(LocationOperand::cast(source)->IsCompatible(
+      LocationOperand::cast(destination)));
+  if (source->IsAnyRegister()) {
+    if (destination->IsAnyRegister()) {
+      return MoveType::kRegisterToRegister;
+    } else {
+      DCHECK(destination->IsAnyStackSlot());
+      return MoveType::kRegisterToStack;
+    }
+  } else {
+    DCHECK(source->IsAnyStackSlot());
+    DCHECK(destination->IsAnyStackSlot());
+    return MoveType::kStackToStack;
+  }
+}
+
 CodeGenerator::CodeGenResult CodeGenerator::AssembleInstruction(
     Instruction* instr, const InstructionBlock* block) {
   int first_unused_stack_slot;

src/compiler/code-generator.h

@@ -224,6 +224,26 @@ class CodeGenerator final : public GapResolver::Assembler {
                                      PushTypeFlags push_type,
                                      ZoneVector<MoveOperands*>* pushes);
 
+  class MoveType {
+   public:
+    enum Type {
+      kRegisterToRegister,
+      kRegisterToStack,
+      kStackToRegister,
+      kStackToStack,
+      kConstantToRegister,
+      kConstantToStack
+    };
+
+    // Detect what type of move or swap needs to be performed. Note that these
+    // functions do not take into account the representation (Tagged, FP,
+    // ...etc).
+
+    static Type InferMove(InstructionOperand* source,
+                          InstructionOperand* destination);
+    static Type InferSwap(InstructionOperand* source,
+                          InstructionOperand* destination);
+  };
   // Called before a tail call |instr|'s gap moves are assembled and allows
   // gap-specific pre-processing, e.g. adjustment of the sp for tail calls that
   // need it before gap moves or conversion of certain gap moves into pushes.

src/compiler/instruction.cc

@@ -96,6 +96,26 @@ bool InstructionOperand::InterferesWith(const InstructionOperand& other) const {
   return false;
 }
 
+bool LocationOperand::IsCompatible(LocationOperand* op) {
+  if (IsRegister() || IsStackSlot()) {
+    return op->IsRegister() || op->IsStackSlot();
+  } else if (kSimpleFPAliasing) {
+    // A backend may choose to generate the same instruction sequence regardless
+    // of the FP representation. As a result, we can relax the compatibility and
+    // allow a Double to be moved in a Float for example. However, this is only
+    // allowed if registers do not overlap.
+    return (IsFPRegister() || IsFPStackSlot()) &&
+           (op->IsFPRegister() || op->IsFPStackSlot());
+  } else if (IsFloatRegister() || IsFloatStackSlot()) {
+    return op->IsFloatRegister() || op->IsFloatStackSlot();
+  } else if (IsDoubleRegister() || IsDoubleStackSlot()) {
+    return op->IsDoubleRegister() || op->IsDoubleStackSlot();
+  } else {
+    return (IsSimd128Register() || IsSimd128StackSlot()) &&
+           (op->IsSimd128Register() || op->IsSimd128StackSlot());
+  }
+}
+
 void InstructionOperand::Print(const RegisterConfiguration* config) const {
   OFStream os(stdout);
   PrintableInstructionOperand wrapper;

src/compiler/instruction.h

@@ -491,6 +491,9 @@ class LocationOperand : public InstructionOperand {
     UNREACHABLE();
   }
 
+  // Return true if the locations can be moved to one another.
+  bool IsCompatible(LocationOperand* op);
+
   static LocationOperand* cast(InstructionOperand* op) {
     DCHECK(op->IsAnyLocationOperand());
     return static_cast<LocationOperand*>(op);