Merged: [wasm-simd][liftoff][arm64] Check offset fits in str immediate

When filling stack slots, the start offset can be too large to fit into
the immediate of a str instruction (which is used to handle remainders
after stp). For example, a function with 32 i64 params will require 256
bytes reserved for the params, so the offset starts at 256 + 16
(instance) = 272. This does not fit into a int9, so we hit an
UNREACHABLE case when emitting str.

The fix here checks that start can fit in an unscaled immediate, and if
it doesn't fallback to the general case. We could use the Str
from macro-asesmbler, but that uses another instruction, so we are not
saving anything.

A check for IsImmLSUnscaled(-start-12) is sufficient because 12 is the
largest possible value for remainder. So if -start-12 fits, everything
else will fit.

[email protected]

(cherry picked from commit c92e74f)

Bug: v8:10645
Change-Id: Ibde69e667693e9c2c267469181db79c91e4aa765
No-Try: true
No-Presubmit: true
No-Tree-Checks: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2274084
Reviewed-by: Zhi An Ng <[email protected]>
Commit-Queue: Zhi An Ng <[email protected]>
Cr-Commit-Position: refs/branch-heads/8.5@{v8#14}
Cr-Branched-From: a7f8bc4-refs/heads/8.5.210@{#1}
Cr-Branched-From: dd58472-refs/heads/master@{#68510}

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

@@ -771,13 +771,21 @@ void LiftoffAssembler::FillI64Half(Register, int offset, RegPairHalf) {
 }
 
 void LiftoffAssembler::FillStackSlotsWithZero(int start, int size) {
+  // Zero 'size' bytes *below* start, byte at offset 'start' is untouched.
+  DCHECK_LE(0, start);
   DCHECK_LT(0, size);
   DCHECK_EQ(0, size % 4);
   RecordUsedSpillOffset(start + size);
 
   int max_stp_offset = -start - size;
+  // We check IsImmLSUnscaled(-start-12) because str only allows for unscaled
+  // 9-bit immediate offset [-256,256]. If start is large enough, which can
+  // happen when a function has many params (>=32 i64), str cannot be encoded
+  // properly. We can use Str, which will generate more instructions, so
+  // fallback to the general case below.
   if (size <= 12 * kStackSlotSize &&
-      IsImmLSPair(max_stp_offset, kXRegSizeLog2)) {
+      IsImmLSPair(max_stp_offset, kXRegSizeLog2) &&
+      IsImmLSUnscaled(-start - 12)) {
     // Special straight-line code for up to 12 slots. Generates one
     // instruction per two slots (<= 7 instructions total).
     STATIC_ASSERT(kStackSlotSize == kSystemPointerSize);

test/cctest/wasm/test-run-wasm-simd-liftoff.cc

@@ -186,6 +186,41 @@ WASM_SIMD_LIFTOFF_TEST(S8x16Shuffle_SingleOperand) {
   }
 }
 
+// Exercise Liftoff's logic for zero-initializing stack slots. We were using an
+// incorrect instruction for storing zeroes into the slot when the slot offset
+// was too large to fit in the instruction as an immediate.
+WASM_SIMD_LIFTOFF_TEST(FillStackSlotsWithZero_CheckStartOffset) {
+  WasmRunner<int64_t> r(ExecutionTier::kLiftoff, kNoLowerSimd);
+  // Function that takes in 32 i64 arguments, returns i64. This gets us a large
+  // enough starting offset from which we spill locals.
+  // start = 32 * 8 + 16 (instance) = 272 (cannot fit in signed int9).
+  FunctionSig* sig =
+      r.CreateSig<int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t,
+                  int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t,
+                  int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t,
+                  int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t,
+                  int64_t, int64_t, int64_t, int64_t, int64_t>();
+  WasmFunctionCompiler& simd_func = r.NewFunction(sig);
+
+  // We zero 16 bytes at a time using stp, so allocate locals such that we get a
+  // remainder, 8 in this case, so we hit the case where we use str.
+  simd_func.AllocateLocal(kWasmS128);
+  simd_func.AllocateLocal(kWasmI64);
+  BUILD(simd_func, WASM_I64V_1(1));
+
+  BUILD(r, WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1), WASM_I64V_1(1),
+        WASM_CALL_FUNCTION0(simd_func.function_index()));
+
+  CHECK_EQ(1, r.Call());
+}
+
 #undef WASM_SIMD_LIFTOFF_TEST
 
 }  // namespace test_run_wasm_simd_liftoff