Reland "[liftoff][arm64] Zero-extend offsets also for SIMD"

This is a reland of b99fe75c.
The test is now skipped on non-SIMD hardware.

Original change's description:
> [liftoff][arm64] Zero-extend offsets also for SIMD
>
> This extends https://crrev.com/c/2917612 also for SIMD, which
> (sometimes) uses the special {GetMemOpWithImmOffsetZero} method.
> As part of this CL, that method is renamed to {GetEffectiveAddress}
> which IMO is a better name. Also, it just returns a register to make the
> semantic of that function obvious in the signature.
>
> Drive-by: When sign extending to 32 bit, only write to the W portion of
>           the register. This is a bit cleaner, and I first thought that
>           this would be the bug.
>
> R=jkummerow@chromium.org
> CC=​thibaudm@chromium.org
>
> Bug: chromium:1231950, v8:12018
> Change-Id: Ifaefe1f18e3a00534a30c99e3c37ed09d9508f6e
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3049073
> Reviewed-by: Zhi An Ng <zhin@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#75898}

TBR=zhin@chromium.org
CC=jkummerow@chromium.org, thibaudm@chromium.org

Bug: chromium:1231950, v8:12018
Change-Id: I662b62fafe99389be7a6c23b970fdf3768f866cf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3051610Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75901}

src/wasm/baseline/arm64/liftoff-assembler-arm64.h

@@ -138,27 +138,23 @@ inline MemOperand GetMemOp(LiftoffAssembler* assm,
                     : MemOperand(effective_addr, offset.W(), UXTW);
 }
 
-// Certain load instructions do not support offset (register or immediate).
-// This creates a MemOperand that is suitable for such instructions by adding
-// |addr|, |offset| (if needed), and |offset_imm| into a temporary.
-inline MemOperand GetMemOpWithImmOffsetZero(LiftoffAssembler* assm,
+// Compute the effective address (sum of |addr|, |offset| (if given) and
+// |offset_imm|) into a temporary register. This is needed for certain load
+// instructions that do not support an offset (register or immediate).
+// Returns |addr| if both |offset| and |offset_imm| are zero.
+inline Register GetEffectiveAddress(LiftoffAssembler* assm,
                                     UseScratchRegisterScope* temps,
                                     Register addr, Register offset,
                                     uintptr_t offset_imm) {
+  if (!offset.is_valid() && offset_imm == 0) return addr;
   Register tmp = temps->AcquireX();
   if (offset.is_valid()) {
-    // offset has passed BoundsCheckMem in liftoff-compiler, and been unsigned
-    // extended, so it is fine to use the full width of the register.
-    assm->Add(tmp, addr, offset);
-    if (offset_imm != 0) {
-      assm->Add(tmp, tmp, offset_imm);
-    }
-  } else {
-    if (offset_imm != 0) {
-      assm->Add(tmp, addr, offset_imm);
-    }
+    // TODO(clemensb): This needs adaption for memory64.
+    assm->Add(tmp, addr, Operand(offset, UXTW));
+    addr = tmp;
   }
-  return MemOperand(tmp.X(), 0);
+  if (offset_imm != 0) assm->Add(tmp, addr, offset_imm);
+  return tmp;
 }
 
 enum class ShiftDirection : bool { kLeft, kRight };
@@ -1501,11 +1497,11 @@ bool LiftoffAssembler::emit_type_conversion(WasmOpcode opcode,
 }
 
 void LiftoffAssembler::emit_i32_signextend_i8(Register dst, Register src) {
-  sxtb(dst, src);
+  sxtb(dst.W(), src.W());
 }
 
 void LiftoffAssembler::emit_i32_signextend_i16(Register dst, Register src) {
-  sxth(dst, src);
+  sxth(dst.W(), src.W());
 }
 
 void LiftoffAssembler::emit_i64_signextend_i8(LiftoffRegister dst,
@@ -1639,8 +1635,8 @@ void LiftoffAssembler::LoadTransform(LiftoffRegister dst, Register src_addr,
   UseScratchRegisterScope temps(this);
   MemOperand src_op =
       transform == LoadTransformationKind::kSplat
-          ? liftoff::GetMemOpWithImmOffsetZero(this, &temps, src_addr,
-                                               offset_reg, offset_imm)
+          ? MemOperand{liftoff::GetEffectiveAddress(this, &temps, src_addr,
+                                                    offset_reg, offset_imm)}
           : liftoff::GetMemOp(this, &temps, src_addr, offset_reg, offset_imm);
   *protected_load_pc = pc_offset();
   MachineType memtype = type.mem_type();
@@ -1691,8 +1687,8 @@ void LiftoffAssembler::LoadLane(LiftoffRegister dst, LiftoffRegister src,
                                 uintptr_t offset_imm, LoadType type,
                                 uint8_t laneidx, uint32_t* protected_load_pc) {
   UseScratchRegisterScope temps(this);
-  MemOperand src_op = liftoff::GetMemOpWithImmOffsetZero(
-      this, &temps, addr, offset_reg, offset_imm);
+  MemOperand src_op{
+      liftoff::GetEffectiveAddress(this, &temps, addr, offset_reg, offset_imm)};
   *protected_load_pc = pc_offset();
 
   MachineType mem_type = type.mem_type();
@@ -1718,8 +1714,8 @@ void LiftoffAssembler::StoreLane(Register dst, Register offset,
                                  StoreType type, uint8_t lane,
                                  uint32_t* protected_store_pc) {
   UseScratchRegisterScope temps(this);
-  MemOperand dst_op =
-      liftoff::GetMemOpWithImmOffsetZero(this, &temps, dst, offset, offset_imm);
+  MemOperand dst_op{
+      liftoff::GetEffectiveAddress(this, &temps, dst, offset, offset_imm)};
   if (protected_store_pc) *protected_store_pc = pc_offset();
 
   MachineRepresentation rep = type.mem_rep();

test/mjsunit/mjsunit.status

@@ -1432,6 +1432,7 @@
   'regress/wasm/regress-1165966': [SKIP],
   'regress/wasm/regress-1187831': [SKIP],
   'regress/wasm/regress-1199662': [SKIP],
+  'regress/wasm/regress-1231950': [SKIP],
 }],  # no_simd_hardware == True
 
 ##############################################################################
@@ -1456,6 +1457,12 @@
   'concurrent-initial-prototype-change-1': [SKIP],
 }],  # variant == concurrent_inlining
 
+##############################################################################
+['variant == instruction_scheduling or variant == stress_instruction_scheduling', {
+  # BUG(12018): This test currently fails with --turbo-instruction-scheduling.
+  'regress/wasm/regress-1231950': [SKIP],
+}],  # variant == instruction_scheduling or variant == stress_instruction_scheduling
+
 ################################################################################
 ['single_generation', {
   # These tests rely on allocation site tracking which only works in the young generation.

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

+// Copyright 2021 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.
+
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+builder.addMemory(1, 1);
+builder.addFunction('main', kSig_d_v)
+    .addBody([
+      ...wasmI32Const(-3),                            // i32.const
+      kExprI32SExtendI8,                              // i32.extend8_s
+      kSimdPrefix, kExprS128Load32Splat, 0x01, 0x02,  // s128.load32_splat
+      kExprUnreachable,                               // unreachable
+    ])
+    .exportFunc();
+const instance = builder.instantiate();
+assertTraps(kTrapMemOutOfBounds, instance.exports.main);