From 982937893e9643d8bf5b8295d5e26ac9a89a7391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 17 Apr 2021 17:34:16 +0200 Subject: [PATCH] deps: V8: cherry-pick f44fcbf803ac MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: Merged: [wasm][liftoff][ia32] Fix register allocation of CompareExchange The register that holds the {new_value} for the AtomicCompareExchange8U has to be a byte register on ia32. There was code to guarantee that, but after that code there was code that frees the {eax} register, and that code moved the {new_value} to a different register again. With this CL we first free {eax}, and then find a byte register for the {new_value}. R=​clemensb@chromium.org NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true (cherry picked from commit 70a389ac8778064e470a95412d40e17f97898142) Bug: chromium:1140549 Change-Id: I1679f3f9ab26c5416ea251c7925366ff43336d85 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2491031 Reviewed-by: Clemens Backes Commit-Queue: Andreas Haas Cr-Original-Commit-Position: refs/heads/master@{#70721} Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504512 Cr-Commit-Position: refs/branch-heads/8.6@{#38} Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1} Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472} Refs: https://github.com/v8/v8/commit/f44fcbf803aca41a4d51ba9614017ea14d03df47 PR-URL: https://github.com/nodejs/node/pull/38275 Reviewed-By: Matteo Collina Reviewed-By: Jiawen Geng Reviewed-By: Shelley Vohr --- common.gypi | 2 +- .../baseline/ia32/liftoff-assembler-ia32.h | 17 +++++++------ .../mjsunit/regress/wasm/regress-1140549.js | 25 +++++++++++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/wasm/regress-1140549.js diff --git a/common.gypi b/common.gypi index 621b8627944482..470cfd6adbcf9d 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.54', + 'v8_embedder_string': '-node.55', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h b/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h index b9466195236cfc..830aa7c4b1d0c8 100644 --- a/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h +++ b/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h @@ -545,6 +545,16 @@ void LiftoffAssembler::AtomicCompareExchange( Register expected_reg = is_64_bit_op ? expected.low_gp() : expected.gp(); Register result_reg = expected_reg; + // The cmpxchg instruction uses eax to store the old value of the + // compare-exchange primitive. Therefore we have to spill the register and + // move any use to another register. + ClearRegister(eax, {&dst_addr, &value_reg}, + LiftoffRegList::ForRegs(dst_addr, value_reg, expected_reg)); + if (expected_reg != eax) { + mov(eax, expected_reg); + expected_reg = eax; + } + bool is_byte_store = type.size() == 1; LiftoffRegList pinned = LiftoffRegList::ForRegs(dst_addr, value_reg, expected_reg); @@ -558,13 +568,6 @@ void LiftoffAssembler::AtomicCompareExchange( pinned.clear(LiftoffRegister(value_reg)); } - // The cmpxchg instruction uses eax to store the old value of the - // compare-exchange primitive. Therefore we have to spill the register and - // move any use to another register. - ClearRegister(eax, {&dst_addr, &value_reg}, pinned); - if (expected_reg != eax) { - mov(eax, expected_reg); - } Operand dst_op = Operand(dst_addr, offset_imm); diff --git a/deps/v8/test/mjsunit/regress/wasm/regress-1140549.js b/deps/v8/test/mjsunit/regress/wasm/regress-1140549.js new file mode 100644 index 00000000000000..65191e1962373c --- /dev/null +++ b/deps/v8/test/mjsunit/regress/wasm/regress-1140549.js @@ -0,0 +1,25 @@ +// 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: --wasm-staging + +load('test/mjsunit/wasm/wasm-module-builder.js'); + +const builder = new WasmModuleBuilder(); +builder.addMemory(16, 32, false, true); +builder.addType(makeSig([], [])); +builder.addFunction(undefined, 0 /* sig */) + .addBodyWithEnd([ +// signature: v_v +// body: +kExprI32Const, 0x00, +kExprI32Const, 0x00, +kExprI32Const, 0x00, +kAtomicPrefix, kExprI32AtomicCompareExchange8U, 0x00, 0xc3, 0x01, +kExprDrop, +kExprEnd, // end @193 +]); +builder.addExport('main', 0); +const instance = builder.instantiate(); +print(instance.exports.main(1, 2, 3));