[turboshaft][wasm] Fix 64-bit atomic load/store on 32-bit arch

jakobkummerow · V8 LUCI CQ · commit e0a43f23ca02 · 2024-01-24T14:34:14.000Z
Int64Lowering currently cannot deal with "elem_size_log2" scaling parameters, because AtomicWord32Pair doesn't support them. So the MachineOptimizationReducer shouldn't produce these parameters on this particular configuration. Fixed: chromium:1520311 Change-Id: I68f5cf48fc2d617bd62aeced018983eb6e77d56c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5232164 Commit-Queue: Nico Hartmann <nicohartmann@chromium.org> Auto-Submit: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Cr-Commit-Position: refs/heads/main@{#91985}
diff --git a/src/compiler/turboshaft/int64-lowering-reducer.h b/src/compiler/turboshaft/int64-lowering-reducer.h
@@ -249,6 +249,10 @@ class Int64LoweringReducer : public Next {
     if (kind.is_atomic) {
       if (loaded_rep == MemoryRepresentation::Int64() ||
           loaded_rep == MemoryRepresentation::Uint64()) {
+        // TODO(jkummerow): Support non-zero scales in AtomicWord32PairOp, and
+        // remove the corresponding bailout in MachineOptimizationReducer to
+        // allow generating them.
+        CHECK_EQ(element_scale, 0);
         return __ AtomicWord32PairLoad(base, index, offset);
       }
       if (result_rep == RegisterRepresentation::Word64()) {
@@ -282,8 +286,10 @@ class Int64LoweringReducer : public Next {
         stored_rep == MemoryRepresentation::Uint64()) {
       auto [low, high] = Unpack(value);
       if (kind.is_atomic) {
-        // Linear wasm doesn't use element_size_log2.
-        DCHECK_EQ(element_size_log2, 0);
+        // TODO(jkummerow): Support non-zero scales in AtomicWord32PairOp, and
+        // remove the corresponding bailout in MachineOptimizationReducer to
+        // allow generating them.
+        CHECK_EQ(element_size_log2, 0);
         return __ AtomicWord32PairStore(base, index, low, high, offset);
       }
       return __ Tuple(
diff --git a/src/compiler/turboshaft/machine-optimization-reducer.h b/src/compiler/turboshaft/machine-optimization-reducer.h
@@ -1700,7 +1700,14 @@ class MachineOptimizationReducer : public Next {
                                maybe_indirect_pointer_tag);
     }
     if (ShouldSkipOptimizationStep()) goto no_change;
-
+#if V8_TARGET_ARCH_32_BIT
+    if (kind.is_atomic && stored_rep.SizeInBytes() == 8) {
+      // AtomicWord32PairOp (as used by Int64Lowering) cannot handle
+      // element_scale != 0 currently.
+      // TODO(jkummerow): Add support for element_scale in AtomicWord32PairOp.
+      goto no_change;
+    }
+#endif
     if (stored_rep.SizeInBytes() <= 4) {
       value = TryRemoveWord32ToWord64Conversion(value);
     }
@@ -1758,6 +1765,14 @@ class MachineOptimizationReducer : public Next {
                               offset, element_scale);
     }
     if (ShouldSkipOptimizationStep()) goto no_change;
+#if V8_TARGET_ARCH_32_BIT
+    if (kind.is_atomic && loaded_rep.SizeInBytes() == 8) {
+      // AtomicWord32PairOp (as used by Int64Lowering) cannot handle
+      // element_scale != 0 currently.
+      // TODO(jkummerow): Add support for element_scale in AtomicWord32PairOp.
+      goto no_change;
+    }
+#endif
 
     while (true) {
       index =
diff --git a/test/mjsunit/regress/wasm/regress-crbug-1520311.js b/test/mjsunit/regress/wasm/regress-crbug-1520311.js
@@ -0,0 +1,41 @@
+// Copyright 2024 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.
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+
+builder.addMemory(1, 1);
+builder.exportMemoryAs("memory");
+
+builder.addFunction("store", kSig_v_i).exportFunc().addBody([
+  kExprLocalGet, 0,
+  kExprI32Const, 3,
+  kExprI32Shl,
+  kExprI64Const, 42,
+  kAtomicPrefix, kExprI64AtomicStore, 3, 0,  // alignment, offset
+]);
+
+builder.addFunction("load", kSig_l_i).exportFunc().addBody([
+  kExprLocalGet, 0,
+  kExprI32Const, 3,
+  kExprI32Shl,
+  kAtomicPrefix, kExprI64AtomicLoad, 3, 0,  // alignment, offset
+]);
+
+let instance = builder.instantiate();
+const kStoreIndex = 1;
+instance.exports.store(kStoreIndex);
+
+let i64 = new BigInt64Array(instance.exports.memory.buffer);
+
+assertEquals(0n, i64[0]);
+assertEquals(42n, i64[kStoreIndex]);
+
+const kLoadIndex = 10;
+const kLoadValue = 1234n;
+i64[kLoadIndex] = kLoadValue;
+let load = instance.exports.load;
+assertEquals(0n, load(kLoadIndex * 8));
+assertEquals(kLoadValue, load(kLoadIndex));
