Skip to content

Commit 1b6d976

Browse files
committed
deps: V8: cherry-pick 9ec4e9095a25
Original commit message: [turbofan] Fix 32-to-64 bit spill slot moves Other optimizations can create a situation where it is valid to treat a stack slot as either 32-bit (which is what its value was created as) or 64-bit value (to which it was implicitly zero-extended). So when moving such a value to a register, we cannot use a 32-bit move instruction just because the source was annotated as such; we must also take the target slot's representation into account. Fixed: chromium:1407594 Bug: chromium:1356461 Change-Id: I00d850c11a020b055e90f6107b604cdd267d9b6c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4197349 Auto-Submit: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Maya Lekova <mslekova@chromium.org> Commit-Queue: Maya Lekova <mslekova@chromium.org> Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/main@{#85501} Refs: v8/v8@9ec4e90
1 parent c95ceef commit 1b6d976

File tree

3 files changed

+83
-7
lines changed

3 files changed

+83
-7
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.16',
39+
'v8_embedder_string': '-node.17',
4040

4141
##### V8 defaults for Node.js #####
4242

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

+18-6
Original file line numberDiff line numberDiff line change
@@ -5172,6 +5172,18 @@ void CodeGenerator::SetPendingMove(MoveOperands* move) {
51725172
}
51735173
}
51745174

5175+
namespace {
5176+
5177+
bool Is32BitOperand(InstructionOperand* operand) {
5178+
DCHECK(operand->IsStackSlot() || operand->IsRegister());
5179+
MachineRepresentation mr = LocationOperand::cast(operand)->representation();
5180+
return mr == MachineRepresentation::kWord32 ||
5181+
mr == MachineRepresentation::kCompressed ||
5182+
mr == MachineRepresentation::kCompressedPointer;
5183+
}
5184+
5185+
} // namespace
5186+
51755187
void CodeGenerator::AssembleMove(InstructionOperand* source,
51765188
InstructionOperand* destination) {
51775189
X64OperandConverter g(this, nullptr);
@@ -5295,18 +5307,18 @@ void CodeGenerator::AssembleMove(InstructionOperand* source,
52955307
case MoveType::kStackToRegister: {
52965308
Operand src = g.ToOperand(source);
52975309
if (source->IsStackSlot()) {
5298-
MachineRepresentation mr =
5299-
LocationOperand::cast(source)->representation();
5300-
const bool is_32_bit = mr == MachineRepresentation::kWord32 ||
5301-
mr == MachineRepresentation::kCompressed ||
5302-
mr == MachineRepresentation::kCompressedPointer;
53035310
// TODO(13581): Fix this for other code kinds (see
53045311
// https://crbug.com/1356461).
5305-
if (code_kind() == CodeKind::WASM_FUNCTION && is_32_bit) {
5312+
if (code_kind() == CodeKind::WASM_FUNCTION && Is32BitOperand(source) &&
5313+
Is32BitOperand(destination)) {
53065314
// When we need only 32 bits, move only 32 bits. Benefits:
53075315
// - Save a byte here and there (depending on the destination
53085316
// register; "movl eax, ..." is smaller than "movq rax, ...").
53095317
// - Safeguard against accidental decompression of compressed slots.
5318+
// We must check both {source} and {destination} to be 32-bit values,
5319+
// because treating 32-bit sources as 64-bit values can be perfectly
5320+
// fine as a result of virtual register renaming (to avoid redundant
5321+
// explicit zero-extensions that also happen implicitly).
53105322
__ movl(g.ToRegister(destination), src);
53115323
} else {
53125324
__ movq(g.ToRegister(destination), src);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright 2023 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
6+
7+
let builder = new WasmModuleBuilder();
8+
builder.addMemory(1, 1, false);
9+
builder.addDataSegment(0, [0x78, 0x56, 0x34, 0x12]);
10+
11+
let spiller = builder.addFunction('spiller', kSig_v_v).addBody([]);
12+
13+
builder.addFunction('main', kSig_l_v)
14+
.exportFunc()
15+
.addLocals(kWasmI64, 1)
16+
.addBody([
17+
// Initialize {var0} to 0x12345678 via a zero-extended 32-bit load.
18+
kExprI32Const, 0,
19+
kExprI64LoadMem32U, 2, 0,
20+
kExprLocalSet, 0,
21+
kExprCallFunction, spiller.index,
22+
// The observable effect of this loop is that {var0} is left-shifted
23+
// until it ends in 0x..000000. The details are specifically crafted
24+
// to recreate a particular pattern of spill slot moves.
25+
kExprLoop, kWasmVoid,
26+
kExprI32Const, 0,
27+
kExprI32LoadMem, 2, 0,
28+
kExprI32Eqz,
29+
// This block is never taken; it only serves to influence register
30+
// allocator choices.
31+
kExprIf, kWasmVoid,
32+
kExprLocalGet, 0,
33+
kExprI64Const, 1,
34+
kExprI64And,
35+
kExprLocalSet, 0,
36+
kExprEnd, // if
37+
kExprLocalGet, 0,
38+
kExprI64Const, 1,
39+
kExprI64And,
40+
kExprI64Eqz,
41+
// This block is always taken; it is conditional in order to influence
42+
// register allocator choices.
43+
kExprIf, kWasmVoid,
44+
kExprLocalGet, 0,
45+
kExprI64Const, 8,
46+
kExprI64Shl,
47+
kExprLocalSet, 0,
48+
kExprEnd, // if
49+
kExprBlock, kWasmVoid,
50+
kExprLocalGet, 0,
51+
...wasmI64Const(0xFFFFFF),
52+
kExprI64And,
53+
kExprI64Eqz,
54+
kExprI32Eqz,
55+
kExprCallFunction, spiller.index,
56+
kExprBrIf, 1,
57+
kExprEnd, // block
58+
kExprCallFunction, spiller.index,
59+
kExprEnd, // loop
60+
kExprLocalGet, 0,
61+
]);
62+
63+
let instance = builder.instantiate();
64+
assertEquals("12345678000000", instance.exports.main().toString(16));

0 commit comments

Comments
 (0)