[wasm-simd][x64][ia32] Do not overwrite input register

We are ovewriting input register (contains the shift) when we are
masking it, instead, move to a temporary,then mask it.

Bug: chromium:1065599
Change-Id: Iab72b94581239447e444746681387350b576e24a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2125941
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66997}

src/compiler/backend/ia32/code-generator-ia32.cc

@@ -501,10 +501,11 @@ class OutOfLineRecordWrite final : public OutOfLineCode {
       __ opcode(dst, dst, static_cast<byte>(i.InputInt##width(1))); \
     } else {                                                        \
       XMMRegister tmp = i.TempSimd128Register(0);                   \
-      Register shift = i.InputRegister(1);                          \
+      Register tmp_shift = i.TempRegister(1);                       \
       constexpr int mask = (1 << width) - 1;                        \
-      __ and_(shift, Immediate(mask));                              \
-      __ Movd(tmp, shift);                                          \
+      __ mov(tmp_shift, i.InputRegister(1));                        \
+      __ and_(tmp_shift, Immediate(mask));                          \
+      __ Movd(tmp, tmp_shift);                                      \
       __ opcode(dst, dst, tmp);                                     \
     }                                                               \
   } while (false)
@@ -3176,18 +3177,20 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
         __ Pshufd(tmp_simd, tmp_simd, 0);
         __ Pand(dst, tmp_simd);
       } else {
-        Register shift = i.InputRegister(1);
         // Take shift value modulo 8.
-        __ and_(shift, 7);
+        __ mov(tmp, i.InputRegister(1));
+        __ and_(tmp, 7);
         // Mask off the unwanted bits before word-shifting.
         __ Pcmpeqw(kScratchDoubleReg, kScratchDoubleReg);
-        __ mov(tmp, shift);
         __ add(tmp, Immediate(8));
         __ Movd(tmp_simd, tmp);
         __ Psrlw(kScratchDoubleReg, kScratchDoubleReg, tmp_simd);
         __ Packuswb(kScratchDoubleReg, kScratchDoubleReg);
         __ Pand(dst, kScratchDoubleReg);
-        __ Movd(tmp_simd, shift);
+        // TODO(zhin): sub here to avoid asking for another temporary register,
+        // examine codegen for other i8x16 shifts, they use less instructions.
+        __ sub(tmp, Immediate(8));
+        __ Movd(tmp_simd, tmp);
         __ Psllw(dst, dst, tmp_simd);
       }
       break;

src/compiler/backend/ia32/instruction-selector-ia32.cc

@@ -314,7 +314,7 @@ void VisitRROSimdShift(InstructionSelector* selector, Node* node,
   } else {
     InstructionOperand operand0 = g.UseUniqueRegister(node->InputAt(0));
     InstructionOperand operand1 = g.UseUniqueRegister(node->InputAt(1));
-    InstructionOperand temps[] = {g.TempSimd128Register()};
+    InstructionOperand temps[] = {g.TempSimd128Register(), g.TempRegister()};
     selector->Emit(opcode, g.DefineSameAsFirst(node), operand0, operand1,
                    arraysize(temps), temps);
   }

src/compiler/backend/x64/code-generator-x64.cc

@@ -608,10 +608,11 @@ void EmitWordLoadPoisoningIfNeeded(CodeGenerator* codegen,
       __ opcode(dst, static_cast<byte>(i.InputInt##width(1))); \
     } else {                                                   \
       XMMRegister tmp = i.TempSimd128Register(0);              \
-      Register shift = i.InputRegister(1);                     \
+      Register tmp_shift = i.TempRegister(1);                  \
       constexpr int mask = (1 << width) - 1;                   \
-      __ andq(shift, Immediate(mask));                         \
-      __ Movq(tmp, shift);                                     \
+      __ movq(tmp_shift, i.InputRegister(1));                  \
+      __ andq(tmp_shift, Immediate(mask));                     \
+      __ Movq(tmp, tmp_shift);                                 \
       __ opcode(dst, tmp);                                     \
     }                                                          \
   } while (false)
@@ -3315,18 +3316,20 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
         __ Pshufd(tmp_simd, tmp_simd, static_cast<uint8_t>(0));
         __ Pand(dst, tmp_simd);
       } else {
-        Register shift = i.InputRegister(1);
         // Mask off the unwanted bits before word-shifting.
         __ Pcmpeqw(kScratchDoubleReg, kScratchDoubleReg);
         // Take shift value modulo 8.
-        __ andq(shift, Immediate(7));
-        __ movq(tmp, shift);
+        __ movq(tmp, i.InputRegister(1));
+        __ andq(tmp, Immediate(7));
         __ addq(tmp, Immediate(8));
         __ Movq(tmp_simd, tmp);
         __ Psrlw(kScratchDoubleReg, tmp_simd);
         __ Packuswb(kScratchDoubleReg, kScratchDoubleReg);
         __ Pand(dst, kScratchDoubleReg);
-        __ Movq(tmp_simd, shift);
+        // TODO(zhin): subq here to avoid asking for another temporary register,
+        // examine codegen for other i8x16 shifts, they use less instructions.
+        __ subq(tmp, Immediate(8));
+        __ Movq(tmp_simd, tmp);
         __ Psllw(dst, tmp_simd);
       }
       break;

src/compiler/backend/x64/instruction-selector-x64.cc

@@ -2815,7 +2815,8 @@ SIMD_TYPES(VISIT_SIMD_REPLACE_LANE)
       Emit(kX64##Opcode, g.DefineSameAsFirst(node),                            \
            g.UseRegister(node->InputAt(0)), g.UseImmediate(node->InputAt(1))); \
     } else {                                                                   \
-      InstructionOperand temps[] = {g.TempSimd128Register()};                  \
+      InstructionOperand temps[] = {g.TempSimd128Register(),                   \
+                                    g.TempRegister()};                         \
       Emit(kX64##Opcode, g.DefineSameAsFirst(node),                            \
            g.UseUniqueRegister(node->InputAt(0)),                              \
            g.UseUniqueRegister(node->InputAt(1)), arraysize(temps), temps);    \

test/mjsunit/regress/wasm/regress-1055692.js

@@ -32,4 +32,4 @@ kExprEnd,  // end @18
 ]);
 builder.addExport('main', 0);
 const instance = builder.instantiate();
-print(instance.exports.main(1, 2, 3));
+instance.exports.main(1, 2, 3);

test/mjsunit/regress/wasm/regress-1065599.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --experimental-wasm-simd
+
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+builder.addMemory(16, 32, false);
+builder.addType(makeSig([kWasmI32, kWasmI32, kWasmI32], [kWasmI32]));
+// Generate function 1 (out of 1).
+builder.addFunction(undefined, 0 /* sig */).addBodyWithEnd([
+  // signature: i_iii
+  // body:
+  kExprI32Const, 0xba, 0x01,       // i32.const
+  kSimdPrefix, kExprI8x16Splat,    // i8x16.splat
+  kExprMemorySize, 0x00,           // memory.size
+  kSimdPrefix, kExprI16x8ShrS,     // i16x8.shr_s
+  kSimdPrefix, kExprS1x16AnyTrue,  // s1x16.any_true
+  kExprMemorySize, 0x00,           // memory.size
+  kExprI32RemS,                    // i32.rem_s
+  kExprEnd,                        // end @15
+]);
+builder.addExport('main', 0);
+const instance = builder.instantiate();
+print(instance.exports.main(1, 2, 3));

test/mjsunit/wasm/wasm-module-builder.js

@@ -469,10 +469,13 @@ let kExprI64AtomicCompareExchange32U = 0x4e;
 // Simd opcodes.
 let kExprS128LoadMem = 0x00;
 let kExprS128StoreMem = 0x01;
+let kExprI8x16Splat = 0x04;
 let kExprI32x4Splat = 0x0c;
 let kExprF32x4Splat = 0x12;
 let kExprI32x4Eq = 0x2c;
+let kExprS1x16AnyTrue = 0x52;
 let kExprS1x8AnyTrue = 0x63;
+let kExprI16x8ShrS = 0x66;
 let kExprS1x4AllTrue = 0x75;
 let kExprI32x4Add = 0x79;
 let kExprF32x4Min = 0x9e;