Commit 402b7f15 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[wasm] Add zero-extension for I64AtomicCompareExchange32U

x64's cmpxchgl instruction does not zero-extend the register. The stale
high word caused the difference in the results of the interpreter and
Liftoff/TurboFan.

R=clemensb@chromium.org
CC=zhin@chromium.org

Bug: chromium:1059529
Change-Id: I0fd440bee26e25b90b29533cfa9151e4d87754e3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2098726
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66685}
parent 8372a7c5
......@@ -4061,6 +4061,8 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
case kX64Word64AtomicCompareExchangeUint32: {
__ lock();
__ cmpxchgl(i.MemoryOperand(2), i.InputRegister(1));
// Zero-extend the 32 bit value to 64 bit.
__ movl(rax, rax);
break;
}
case kX64Word64AtomicCompareExchangeUint64: {
......
......@@ -639,31 +639,38 @@ void LiftoffAssembler::AtomicCompareExchange(
case StoreType::kI32Store8:
case StoreType::kI64Store8: {
cmpxchgb(dst_op, value_reg);
movzxbq(rax, rax);
movzxbq(result.gp(), rax);
break;
}
case StoreType::kI32Store16:
case StoreType::kI64Store16: {
cmpxchgw(dst_op, value_reg);
movzxwq(rax, rax);
movzxwq(result.gp(), rax);
break;
}
case StoreType::kI32Store: {
cmpxchgl(dst_op, value_reg);
if (result.gp() != rax) {
movl(result.gp(), rax);
}
break;
}
case StoreType::kI32Store:
case StoreType::kI64Store32: {
cmpxchgl(dst_op, value_reg);
// Zero extension.
movl(result.gp(), rax);
break;
}
case StoreType::kI64Store: {
cmpxchgq(dst_op, value_reg);
if (result.gp() != rax) {
movq(result.gp(), rax);
}
break;
}
default:
UNREACHABLE();
}
if (result.gp() != rax) {
movq(result.gp(), rax);
}
}
void LiftoffAssembler::AtomicFence() { mfence(); }
......
......@@ -781,6 +781,23 @@ WASM_EXEC_TEST(I64AtomicCompareExchangeUseOnlyHighWord) {
WASM_I64V(32))));
CHECK_EQ(0x12345678, r.Call());
}
WASM_EXEC_TEST(I64AtomicCompareExchange32UZeroExtended) {
EXPERIMENTAL_FLAG_SCOPE(threads);
WasmRunner<uint32_t> r(execution_tier);
uint64_t* memory =
r.builder().AddMemoryElems<uint64_t>(kWasmPageSize / sizeof(uint64_t));
memory[1] = 0;
r.builder().SetHasSharedMemory();
// Test that the high word of the expected value is cleared in the return
// value.
BUILD(r, WASM_I64_EQZ(WASM_ATOMICS_TERNARY_OP(
kExprI64AtomicCompareExchange32U, WASM_I32V(8),
WASM_I64V(0x1234567800000000), WASM_I64V(0),
MachineRepresentation::kWord32)));
CHECK_EQ(1, r.Call());
}
} // namespace test_run_wasm_atomics_64
} // namespace wasm
} // namespace internal
......
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