diff --git a/src/compiler/backend/x64/code-generator-x64.cc b/src/compiler/backend/x64/code-generator-x64.cc index 6390e33583..00c976d986 100644 --- a/src/compiler/backend/x64/code-generator-x64.cc +++ b/src/compiler/backend/x64/code-generator-x64.cc @@ -5220,6 +5220,18 @@ void CodeGenerator::SetPendingMove(MoveOperands* move) { } } +namespace { + +bool Is32BitOperand(InstructionOperand* operand) { + DCHECK(operand->IsStackSlot() || operand->IsRegister()); + MachineRepresentation mr = LocationOperand::cast(operand)->representation(); + return mr == MachineRepresentation::kWord32 || + mr == MachineRepresentation::kCompressed || + mr == MachineRepresentation::kCompressedPointer; +} + +} // namespace + void CodeGenerator::AssembleMove(InstructionOperand* source, InstructionOperand* destination) { X64OperandConverter g(this, nullptr); @@ -5343,18 +5355,18 @@ void CodeGenerator::AssembleMove(InstructionOperand* source, case MoveType::kStackToRegister: { Operand src = g.ToOperand(source); if (source->IsStackSlot()) { - MachineRepresentation mr = - LocationOperand::cast(source)->representation(); - const bool is_32_bit = mr == MachineRepresentation::kWord32 || - mr == MachineRepresentation::kCompressed || - mr == MachineRepresentation::kCompressedPointer; // TODO(13581): Fix this for other code kinds (see // https://crbug.com/1356461). - if (code_kind() == CodeKind::WASM_FUNCTION && is_32_bit) { + if (code_kind() == CodeKind::WASM_FUNCTION && Is32BitOperand(source) && + Is32BitOperand(destination)) { // When we need only 32 bits, move only 32 bits. Benefits: // - Save a byte here and there (depending on the destination // register; "movl eax, ..." is smaller than "movq rax, ..."). // - Safeguard against accidental decompression of compressed slots. + // We must check both {source} and {destination} to be 32-bit values, + // because treating 32-bit sources as 64-bit values can be perfectly + // fine as a result of virtual register renaming (to avoid redundant + // explicit zero-extensions that also happen implicitly). __ movl(g.ToRegister(destination), src); } else { __ movq(g.ToRegister(destination), src); diff --git a/test/mjsunit/regress/wasm/regress-crbug-1407594.js b/test/mjsunit/regress/wasm/regress-crbug-1407594.js new file mode 100644 index 0000000000..943ba85be4 --- /dev/null +++ b/test/mjsunit/regress/wasm/regress-crbug-1407594.js @@ -0,0 +1,64 @@ +// Copyright 2023 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'); + +let builder = new WasmModuleBuilder(); +builder.addMemory(1, 1, false); +builder.addDataSegment(0, [0x78, 0x56, 0x34, 0x12]); + +let spiller = builder.addFunction('spiller', kSig_v_v).addBody([]); + +builder.addFunction('main', kSig_l_v) + .exportFunc() + .addLocals(kWasmI64, 1) + .addBody([ + // Initialize {var0} to 0x12345678 via a zero-extended 32-bit load. + kExprI32Const, 0, + kExprI64LoadMem32U, 2, 0, + kExprLocalSet, 0, + kExprCallFunction, spiller.index, + // The observable effect of this loop is that {var0} is left-shifted + // until it ends in 0x..000000. The details are specifically crafted + // to recreate a particular pattern of spill slot moves. + kExprLoop, kWasmVoid, + kExprI32Const, 0, + kExprI32LoadMem, 2, 0, + kExprI32Eqz, + // This block is never taken; it only serves to influence register + // allocator choices. + kExprIf, kWasmVoid, + kExprLocalGet, 0, + kExprI64Const, 1, + kExprI64And, + kExprLocalSet, 0, + kExprEnd, // if + kExprLocalGet, 0, + kExprI64Const, 1, + kExprI64And, + kExprI64Eqz, + // This block is always taken; it is conditional in order to influence + // register allocator choices. + kExprIf, kWasmVoid, + kExprLocalGet, 0, + kExprI64Const, 8, + kExprI64Shl, + kExprLocalSet, 0, + kExprEnd, // if + kExprBlock, kWasmVoid, + kExprLocalGet, 0, + ...wasmI64Const(0xFFFFFF), + kExprI64And, + kExprI64Eqz, + kExprI32Eqz, + kExprCallFunction, spiller.index, + kExprBrIf, 1, + kExprEnd, // block + kExprCallFunction, spiller.index, + kExprEnd, // loop + kExprLocalGet, 0, + ]); + +let instance = builder.instantiate(); +assertEquals("12345678000000", instance.exports.main().toString(16));