Revert of [Turbofan] CodeGenerator for ARM avoids moves from VFP to general...

Revert of [Turbofan] CodeGenerator for ARM avoids moves from VFP to general regs. (patchset #4 id:60001 of https://codereview.chromium.org/2497483002/ )

Reason for revert:
This was a speculative fix for perf regressions on Nexus 10 and ChromeOS. However, perf graphs after this landed show no improvement, so we should go back to the smaller, simpler code before.

Original issue's description:
> [Turbofan] CodeGenerator for ARM avoids moves from VFP to general regs.
> - Adds VmovExtended, VswpExtended methods to MacroAssembler. These methods
> use only VFP registers to perform s-register moves.
>
> LOG=N
> BUG=v8:4124

TBR=bmeurer@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=v8:4124

Review-Url: https://codereview.chromium.org/2505003002
Cr-Commit-Position: refs/heads/master@{#41039}

src/arm/macro-assembler-arm.cc

@@ -1051,151 +1051,66 @@ void MacroAssembler::VmovLow(DwVfpRegister dst, Register src) {
   }
 }
 
-void MacroAssembler::VmovExtended(int dst_code, int src_code) {
-  if (src_code < 32 && dst_code < 32) {
-    // src and dst are both s-registers.
-    vmov(SwVfpRegister::from_code(dst_code),
-         SwVfpRegister::from_code(src_code));
-  } else if (src_code < 32) {
-    // src is s-register, dst is in high d-register. Move dst into scratch
-    // d-register to do the s-register move, then back.
-    DCHECK_GT(64, dst_code);
-    DwVfpRegister dst_reg = DwVfpRegister::from_code(dst_code / 2);
-    int dst_s_code = kScratchDoubleReg.low().code() + (dst_code & 1);
-    vmov(kScratchDoubleReg, dst_reg);
-    vmov(SwVfpRegister::from_code(dst_s_code),
-         SwVfpRegister::from_code(src_code));
-    vmov(dst_reg, kScratchDoubleReg);
-  } else if (dst_code < 32) {
-    // src is in high d-register, dst is an s-register. Move src into scratch
-    // d-register, do the s-register move.
-    DCHECK_GT(64, src_code);
-    DwVfpRegister src_reg = DwVfpRegister::from_code(src_code / 2);
-    int src_s_code = kScratchDoubleReg.low().code() + (src_code & 1);
-    vmov(kScratchDoubleReg, src_reg);
-    vmov(SwVfpRegister::from_code(dst_code),
-         SwVfpRegister::from_code(src_s_code));
+void MacroAssembler::VmovExtended(Register dst, int src_code) {
+  DCHECK_LE(32, src_code);
+  DCHECK_GT(64, src_code);
+  if (src_code & 0x1) {
+    VmovHigh(dst, DwVfpRegister::from_code(src_code / 2));
   } else {
-    // src and dst are in high d-registers. Move both into free registers,
-    // do the s-register move, then move dst back.
-    DCHECK_GT(64, src_code);
-    DCHECK_GT(64, dst_code);
-    DwVfpRegister dst_reg = DwVfpRegister::from_code(dst_code / 2);
-    DwVfpRegister src_reg = DwVfpRegister::from_code(src_code / 2);
-    int dst_s_code = kScratchDoubleReg.low().code() + (dst_code & 1);
-    int src_s_code = kDoubleRegZero.low().code() + (src_code & 1);
-    vmov(kScratchDoubleReg, dst_reg);
-    vmov(kDoubleRegZero, src_reg);
-    vmov(SwVfpRegister::from_code(dst_s_code),
-         SwVfpRegister::from_code(src_s_code));
-    vmov(dst_reg, kScratchDoubleReg);
-    vmov(kDoubleRegZero, 0.0);  // restore zero register
+    VmovLow(dst, DwVfpRegister::from_code(src_code / 2));
   }
 }
 
-void MacroAssembler::VmovExtended(int dst_code, const MemOperand& src) {
-  if (dst_code < 32) {
-    vldr(SwVfpRegister::from_code(dst_code), src);
+void MacroAssembler::VmovExtended(int dst_code, Register src) {
+  DCHECK_LE(32, dst_code);
+  DCHECK_GT(64, dst_code);
+  if (dst_code & 0x1) {
+    VmovHigh(DwVfpRegister::from_code(dst_code / 2), src);
   } else {
-    // dst is in high d-register, move it down, load src, then move it back up.
-    DCHECK_GT(64, dst_code);
-    DwVfpRegister dst_reg = DwVfpRegister::from_code(dst_code / 2);
-    int dst_s_code = kScratchDoubleReg.low().code() + (dst_code & 1);
-    vmov(kScratchDoubleReg, dst_reg);
-    vldr(SwVfpRegister::from_code(dst_s_code), src);
-    vmov(dst_reg, kScratchDoubleReg);
-  }
-}
-void MacroAssembler::VmovExtended(const MemOperand& dst, int src_code) {
-  if (src_code < 32) {
-    vstr(SwVfpRegister::from_code(src_code), dst);
-  } else {
-    // src is in high d-register, move it down, store src.
-    DCHECK_GT(64, src_code);
-    DwVfpRegister src_reg = DwVfpRegister::from_code(src_code / 2);
-    int src_s_code = kScratchDoubleReg.low().code() + (src_code & 1);
-    vmov(kScratchDoubleReg, src_reg);
-    vstr(SwVfpRegister::from_code(src_s_code), dst);
+    VmovLow(DwVfpRegister::from_code(dst_code / 2), src);
   }
 }
 
-void MacroAssembler::VswpExtended(int dst_code, int src_code) {
+void MacroAssembler::VmovExtended(int dst_code, int src_code,
+                                  Register scratch) {
   if (src_code < 32 && dst_code < 32) {
     // src and dst are both s-registers.
-    vmov(kScratchDoubleReg.low(), SwVfpRegister::from_code(dst_code));
     vmov(SwVfpRegister::from_code(dst_code),
          SwVfpRegister::from_code(src_code));
-    vmov(SwVfpRegister::from_code(src_code), kScratchDoubleReg.low());
   } else if (src_code < 32) {
-    // src is s-register, dst is in high d-register. Move dst into scratch
-    // d-register to do the s-register swap, then back.
-    DCHECK_GT(64, dst_code);
-    DwVfpRegister dst_reg = DwVfpRegister::from_code(dst_code / 2);
-    int dst_s_code = kScratchDoubleReg.low().code() + (dst_code & 1);
-    int dst_s_temp = kDoubleRegZero.low().code() + (dst_code & 1);
-    vmov(kScratchDoubleReg, dst_reg);
-    vmov(kDoubleRegZero, dst_reg);
-    vmov(SwVfpRegister::from_code(dst_s_code),
-         SwVfpRegister::from_code(src_code));
-    vmov(dst_reg, kScratchDoubleReg);
-    vmov(SwVfpRegister::from_code(src_code),
-         SwVfpRegister::from_code(dst_s_temp));
-    vmov(kDoubleRegZero, 0.0);  // restore zero register
+    // src is an s-register.
+    vmov(scratch, SwVfpRegister::from_code(src_code));
+    VmovExtended(dst_code, scratch);
   } else if (dst_code < 32) {
-    // src is in high d-register, dst is an s-register. Move src into scratch
-    // d-register, do the s-register swap.
-    DCHECK_GT(64, src_code);
-    DwVfpRegister src_reg = DwVfpRegister::from_code(src_code / 2);
-    int src_s_code = kScratchDoubleReg.low().code() + (src_code & 1);
-    int src_s_temp = kDoubleRegZero.low().code() + (src_code & 1);
-    vmov(kScratchDoubleReg, src_reg);
-    vmov(kDoubleRegZero, src_reg);
-    vmov(SwVfpRegister::from_code(src_s_code),
-         SwVfpRegister::from_code(dst_code));
-    vmov(src_reg, kScratchDoubleReg);
-    vmov(SwVfpRegister::from_code(dst_code),
-         SwVfpRegister::from_code(src_s_temp));
-    vmov(kDoubleRegZero, 0.0);  // restore zero register
+    // dst is an s-register.
+    VmovExtended(scratch, src_code);
+    vmov(SwVfpRegister::from_code(dst_code), scratch);
   } else {
-    // src and dst are in high d-registers. Move both into free registers,
-    // do the s-register swap, then move both back.
+    // Neither src or dst are s-registers.
     DCHECK_GT(64, src_code);
     DCHECK_GT(64, dst_code);
-    DwVfpRegister dst_reg = DwVfpRegister::from_code(dst_code / 2);
-    DwVfpRegister src_reg = DwVfpRegister::from_code(src_code / 2);
-    int dst_s_code = kScratchDoubleReg.low().code() + (dst_code & 1);
-    int src_s_code = kDoubleRegZero.low().code() + (src_code & 1);
-    vmov(kScratchDoubleReg, dst_reg);
-    vmov(kDoubleRegZero, src_reg);
-    vmov(SwVfpRegister::from_code(src_s_code ^ 1),
-         SwVfpRegister::from_code(dst_s_code));
-    vmov(SwVfpRegister::from_code(dst_s_code),
-         SwVfpRegister::from_code(src_s_code));
-    vmov(dst_reg, kScratchDoubleReg);
-    vmov(kScratchDoubleReg, src_reg);
-    vmov(SwVfpRegister::from_code(dst_s_code),
-         SwVfpRegister::from_code(src_s_code ^ 1));
-    vmov(src_reg, kScratchDoubleReg);
-    vmov(kDoubleRegZero, 0.0);  // restore zero register
-  }
-}
-
-void MacroAssembler::VswpExtended(const MemOperand& dst, int src_code) {
-  if (src_code < 32) {
-    vldr(kScratchDoubleReg.low(), dst);
-    vstr(SwVfpRegister::from_code(src_code), dst);
-    vmov(SwVfpRegister::from_code(src_code), kScratchDoubleReg.low());
+    VmovExtended(scratch, src_code);
+    VmovExtended(dst_code, scratch);
+  }
+}
+
+void MacroAssembler::VmovExtended(int dst_code, const MemOperand& src,
+                                  Register scratch) {
+  if (dst_code >= 32) {
+    ldr(scratch, src);
+    VmovExtended(dst_code, scratch);
   } else {
-    // src is in high d-register, move it down, do the swap, move src back up.
-    DCHECK_GT(64, src_code);
-    DwVfpRegister src_reg = DwVfpRegister::from_code(src_code / 2);
-    int src_s_code = kScratchDoubleReg.low().code() + (src_code & 1);
-    vmov(kScratchDoubleReg, src_reg);
-    vldr(kDoubleRegZero.low(), dst);
-    vstr(SwVfpRegister::from_code(src_s_code), dst);
-    vmov(SwVfpRegister::from_code(src_s_code), kDoubleRegZero.low());
-    vmov(src_reg, kScratchDoubleReg);
-    vmov(kDoubleRegZero, 0.0);  // restore zero register
+    vldr(SwVfpRegister::from_code(dst_code), src);
+  }
+}
+
+void MacroAssembler::VmovExtended(const MemOperand& dst, int src_code,
+                                  Register scratch) {
+  if (src_code >= 32) {
+    VmovExtended(scratch, src_code);
+    str(scratch, dst);
+  } else {
+    vstr(SwVfpRegister::from_code(src_code), dst);
   }
 }
 

src/arm/macro-assembler-arm.h

@@ -549,12 +549,13 @@ class MacroAssembler: public Assembler {
   void VmovLow(Register dst, DwVfpRegister src);
   void VmovLow(DwVfpRegister dst, Register src);
 
-  // Simulate s-register moves and swaps for imaginary s32 - s63 registers.
-  void VmovExtended(int dst_code, int src_code);
-  void VmovExtended(int dst_code, const MemOperand& src);
-  void VmovExtended(const MemOperand& dst, int src_code);
-  void VswpExtended(int dst_code, int src_code);
-  void VswpExtended(const MemOperand& dst, int src_code);
+  // Simulate s-register moves for imaginary s32 - s63 registers.
+  void VmovExtended(Register dst, int src_code);
+  void VmovExtended(int dst_code, Register src);
+  // Move between s-registers and imaginary s-registers.
+  void VmovExtended(int dst_code, int src_code, Register scratch);
+  void VmovExtended(int dst_code, const MemOperand& src, Register scratch);
+  void VmovExtended(const MemOperand& dst, int src_code, Register scratch);
 
   void LslPair(Register dst_low, Register dst_high, Register src_low,
                Register src_high, Register scratch, Register shift);

src/compiler/arm/code-generator-arm.cc

@@ -1898,10 +1898,10 @@ void CodeGenerator::AssembleMove(InstructionOperand* source,
       int src_code = LocationOperand::cast(source)->register_code();
       if (destination->IsFloatRegister()) {
         int dst_code = LocationOperand::cast(destination)->register_code();
-        __ VmovExtended(dst_code, src_code);
+        __ VmovExtended(dst_code, src_code, kScratchReg);
       } else {
         DCHECK(destination->IsFloatStackSlot());
-        __ VmovExtended(g.ToMemOperand(destination), src_code);
+        __ VmovExtended(g.ToMemOperand(destination), src_code, kScratchReg);
       }
     }
   } else if (source->IsFPStackSlot()) {
@@ -1916,7 +1916,7 @@ void CodeGenerator::AssembleMove(InstructionOperand* source,
         // GapResolver may give us reg codes that don't map to actual
         // s-registers. Generate code to work around those cases.
         int dst_code = LocationOperand::cast(destination)->register_code();
-        __ VmovExtended(dst_code, src);
+        __ VmovExtended(dst_code, src, kScratchReg);
       }
     } else {
       DCHECK(destination->IsFPStackSlot());
@@ -1988,11 +1988,15 @@ void CodeGenerator::AssembleSwap(InstructionOperand* source,
       int src_code = LocationOperand::cast(source)->register_code();
       if (destination->IsFPRegister()) {
         int dst_code = LocationOperand::cast(destination)->register_code();
-        __ VswpExtended(dst_code, src_code);
+        __ VmovExtended(temp.low().code(), src_code, kScratchReg);
+        __ VmovExtended(src_code, dst_code, kScratchReg);
+        __ VmovExtended(dst_code, temp.low().code(), kScratchReg);
       } else {
         DCHECK(destination->IsFPStackSlot());
         MemOperand dst = g.ToMemOperand(destination);
-        __ VswpExtended(dst, src_code);
+        __ VmovExtended(temp.low().code(), src_code, kScratchReg);
+        __ VmovExtended(src_code, dst, kScratchReg);
+        __ vstr(temp.low(), dst);
       }
     }
   } else if (source->IsFPStackSlot()) {