Skip to content

Commit 1ac639a

Browse files
kleisaukedanielleadams
authored andcommitted
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 <[email protected]> Reviewed-by: Maya Lekova <[email protected]> Commit-Queue: Maya Lekova <[email protected]> Commit-Queue: Jakob Kummerow <[email protected]> Cr-Commit-Position: refs/heads/main@{#85501} Refs: v8/v8@9ec4e90 PR-URL: #47092 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>
1 parent d60d4c1 commit 1ac639a

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.25',
39+
'v8_embedder_string': '-node.26',
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
@@ -4916,6 +4916,18 @@ void CodeGenerator::IncrementStackAccessCounter(
49164916
}
49174917
}
49184918

4919+
namespace {
4920+
4921+
bool Is32BitOperand(InstructionOperand* operand) {
4922+
DCHECK(operand->IsStackSlot() || operand->IsRegister());
4923+
MachineRepresentation mr = LocationOperand::cast(operand)->representation();
4924+
return mr == MachineRepresentation::kWord32 ||
4925+
mr == MachineRepresentation::kCompressed ||
4926+
mr == MachineRepresentation::kCompressedPointer;
4927+
}
4928+
4929+
} // namespace
4930+
49194931
void CodeGenerator::AssembleMove(InstructionOperand* source,
49204932
InstructionOperand* destination) {
49214933
X64OperandConverter g(this, nullptr);
@@ -5032,18 +5044,18 @@ void CodeGenerator::AssembleMove(InstructionOperand* source,
50325044
case MoveType::kStackToRegister: {
50335045
Operand src = g.ToOperand(source);
50345046
if (source->IsStackSlot()) {
5035-
MachineRepresentation mr =
5036-
LocationOperand::cast(source)->representation();
5037-
const bool is_32_bit = mr == MachineRepresentation::kWord32 ||
5038-
mr == MachineRepresentation::kCompressed ||
5039-
mr == MachineRepresentation::kCompressedPointer;
50405047
// TODO(13581): Fix this for other code kinds (see
50415048
// https://crbug.com/1356461).
5042-
if (code_kind() == CodeKind::WASM_FUNCTION && is_32_bit) {
5049+
if (code_kind() == CodeKind::WASM_FUNCTION && Is32BitOperand(source) &&
5050+
Is32BitOperand(destination)) {
50435051
// When we need only 32 bits, move only 32 bits. Benefits:
50445052
// - Save a byte here and there (depending on the destination
50455053
// register; "movl eax, ..." is smaller than "movq rax, ...").
50465054
// - Safeguard against accidental decompression of compressed slots.
5055+
// We must check both {source} and {destination} to be 32-bit values,
5056+
// because treating 32-bit sources as 64-bit values can be perfectly
5057+
// fine as a result of virtual register renaming (to avoid redundant
5058+
// explicit zero-extensions that also happen implicitly).
50475059
__ movl(g.ToRegister(destination), src);
50485060
} else {
50495061
__ 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)