Commit 037200e6 authored by eholk's avatar eholk Committed by Commit bot

[wasm] Fix codegen issue for i64.add and i64.sub on ia32

The IA32AddPair and IA32SubPair instructions were using an input register as a
temporary value, which led to registers sometimes being clobbered when they
shouldn't have been. This led to problems, for example, in calling printf to
format doubles:

printf("%f", 1.2345) => 0.61725 (on x86)

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5800

Review-Url: https://codereview.chromium.org/2637583002
Cr-Commit-Position: refs/heads/master@{#42486}
parent 9f545ea9
...@@ -891,10 +891,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction( ...@@ -891,10 +891,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
} else { } else {
__ add(i.OutputRegister(0), i.InputRegister(2)); __ add(i.OutputRegister(0), i.InputRegister(2));
} }
__ adc(i.InputRegister(1), Operand(i.InputRegister(3)));
if (i.OutputRegister(1).code() != i.InputRegister(1).code()) { if (i.OutputRegister(1).code() != i.InputRegister(1).code()) {
__ Move(i.OutputRegister(1), i.InputRegister(1)); __ Move(i.OutputRegister(1), i.InputRegister(1));
} }
__ adc(i.OutputRegister(1), Operand(i.InputRegister(3)));
if (use_temp) { if (use_temp) {
__ Move(i.OutputRegister(0), i.TempRegister(0)); __ Move(i.OutputRegister(0), i.TempRegister(0));
} }
...@@ -916,10 +916,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction( ...@@ -916,10 +916,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
} else { } else {
__ sub(i.OutputRegister(0), i.InputRegister(2)); __ sub(i.OutputRegister(0), i.InputRegister(2));
} }
__ sbb(i.InputRegister(1), Operand(i.InputRegister(3)));
if (i.OutputRegister(1).code() != i.InputRegister(1).code()) { if (i.OutputRegister(1).code() != i.InputRegister(1).code()) {
__ Move(i.OutputRegister(1), i.InputRegister(1)); __ Move(i.OutputRegister(1), i.InputRegister(1));
} }
__ sbb(i.OutputRegister(1), Operand(i.InputRegister(3)));
if (use_temp) { if (use_temp) {
__ Move(i.OutputRegister(0), i.TempRegister(0)); __ Move(i.OutputRegister(0), i.TempRegister(0));
} }
......
...@@ -136,6 +136,21 @@ WASM_EXEC_TEST(I64Add) { ...@@ -136,6 +136,21 @@ WASM_EXEC_TEST(I64Add) {
} }
} }
// The i64 add and subtract regression tests need a 64-bit value with a non-zero
// upper half. This upper half was clobbering eax, leading to the function
// returning 1 rather than 0.
const int64_t kHasBit33On = 0x100000000;
WASM_EXEC_TEST(Regress5800_Add) {
REQUIRE(I64Add);
WasmRunner<int32_t> r(execution_mode);
BUILD(r, WASM_BLOCK(WASM_BR_IF(0, WASM_I64_EQZ(WASM_I64_ADD(
WASM_I64V(0), WASM_I64V(kHasBit33On)))),
WASM_RETURN1(WASM_I32V(0))),
WASM_I32V(0));
CHECK_EQ(0, r.Call());
}
WASM_EXEC_TEST(I64Sub) { WASM_EXEC_TEST(I64Sub) {
REQUIRE(I64Sub); REQUIRE(I64Sub);
WasmRunner<int64_t, int64_t, int64_t> r(execution_mode); WasmRunner<int64_t, int64_t, int64_t> r(execution_mode);
...@@ -145,6 +160,16 @@ WASM_EXEC_TEST(I64Sub) { ...@@ -145,6 +160,16 @@ WASM_EXEC_TEST(I64Sub) {
} }
} }
WASM_EXEC_TEST(Regress5800_Sub) {
REQUIRE(I64Sub);
WasmRunner<int32_t> r(execution_mode);
BUILD(r, WASM_BLOCK(WASM_BR_IF(0, WASM_I64_EQZ(WASM_I64_SUB(
WASM_I64V(0), WASM_I64V(kHasBit33On)))),
WASM_RETURN1(WASM_I32V(0))),
WASM_I32V(0));
CHECK_EQ(0, r.Call());
}
WASM_EXEC_TEST(I64AddUseOnlyLowWord) { WASM_EXEC_TEST(I64AddUseOnlyLowWord) {
REQUIRE(I64Add); REQUIRE(I64Add);
REQUIRE(I32ConvertI64); REQUIRE(I32ConvertI64);
......
// Copyright 2017 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.
load("test/mjsunit/wasm/wasm-constants.js");
load("test/mjsunit/wasm/wasm-module-builder.js");
(function AddTest() {
let builder = new WasmModuleBuilder();
builder.addFunction("main", kSig_i_v)
.addBody([
kExprBlock, kWasmStmt,
kExprI64Const, 0,
// 0x80 ... 0x10 is the LEB encoding of 0x100000000. This is chosen so
// that the 64-bit constant has a non-zero top half. In this bug, the
// top half was clobbering eax, leading to the function return 1 rather
// than 0.
kExprI64Const, 0x80, 0x80, 0x80, 0x80, 0x10,
kExprI64Add,
kExprI64Eqz,
kExprBrIf, 0,
kExprI32Const, 0,
kExprReturn,
kExprEnd,
kExprI32Const, 0
])
.exportFunc();
let module = builder.instantiate();
assertEquals(0, module.exports.main());
})();
(function SubTest() {
let builder = new WasmModuleBuilder();
builder.addFunction("main", kSig_i_v)
.addBody([
kExprBlock, kWasmStmt,
kExprI64Const, 0,
// 0x80 ... 0x10 is the LEB encoding of 0x100000000. This is chosen so
// that the 64-bit constant has a non-zero top half. In this bug, the
// top half was clobbering eax, leading to the function return 1 rather
// than 0.
kExprI64Const, 0x80, 0x80, 0x80, 0x80, 0x10,
kExprI64Sub,
kExprI64Eqz,
kExprBrIf, 0,
kExprI32Const, 0,
kExprReturn,
kExprEnd,
kExprI32Const, 0
])
.exportFunc();
let module = builder.instantiate();
assertEquals(0, module.exports.main());
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment