Commit 6210bc82 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[wasm] Change liftoff assembler interface for atomic binops

The existing interface assumed that for atomic binop instructions, the
value register and the result register are the same. However, for x64,
this assumption is not always useful, and for platforms like arm, this
assumption is even negative.

The existing interface was originally introduced because ia32 lacks
registers, and we wanted to avoid platform-specific code in
liftoff-compiler.cc. However, by now the lack of registers on ia32
required us to use platform-specific code also in other places, so
we can also use it for atomic binops and thereby enable a better code
generation.

R=clemensb@chromium.org

Bug: v8:10108
Change-Id: If39cc5f49934422b632bb2a5793c7f5d5d2b65c0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2150585Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67173}
parent 038e72ea
......@@ -630,37 +630,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -354,37 +354,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -491,37 +491,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -478,28 +478,28 @@ class LiftoffAssembler : public TurboAssembler {
StoreType type, LiftoffRegList pinned);
inline void AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister result,
StoreType type);
uint32_t offset_imm, LiftoffRegister value,
LiftoffRegister result, StoreType type);
inline void AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister result,
StoreType type);
uint32_t offset_imm, LiftoffRegister value,
LiftoffRegister result, StoreType type);
inline void AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister result,
StoreType type);
uint32_t offset_imm, LiftoffRegister value,
LiftoffRegister result, StoreType type);
inline void AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister result,
StoreType type);
uint32_t offset_imm, LiftoffRegister value,
LiftoffRegister result, StoreType type);
inline void AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister result,
StoreType type);
uint32_t offset_imm, LiftoffRegister value,
LiftoffRegister result, StoreType type);
inline void AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister result,
StoreType type);
uint32_t offset_imm, LiftoffRegister value,
LiftoffRegister result, StoreType type);
inline void AtomicCompareExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
......
......@@ -2637,10 +2637,12 @@ class LiftoffCompiler {
const MemoryAccessImmediate<validate>& imm,
void (LiftoffAssembler::*emit_fn)(Register, Register,
uint32_t, LiftoffRegister,
LiftoffRegister,
StoreType)) {
ValueType result_type = type.value_type();
LiftoffRegList pinned;
LiftoffRegister value = pinned.set(__ PopToRegister());
#ifdef V8_TARGET_ARCH_IA32
// We have to reuse the value register as the result register so that we
// don't run out of registers on ia32. For this we use the value register
// as the result register if it has no other uses. Otherwise we allocate
......@@ -2650,7 +2652,12 @@ class LiftoffCompiler {
result = pinned.set(__ GetUnusedRegister(value.reg_class(), pinned));
__ Move(result, value, result_type);
pinned.clear(value);
value = result;
}
#else
LiftoffRegister result =
pinned.set(__ GetUnusedRegister(value.reg_class(), pinned));
#endif
Register index = pinned.set(__ PopToRegister(pinned)).gp();
if (BoundsCheckMem(decoder, type.size(), imm.offset, index, pinned,
kDoForceCheck)) {
......@@ -2663,7 +2670,7 @@ class LiftoffCompiler {
Register addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp();
LOAD_INSTANCE_FIELD(addr, MemoryStart, kSystemPointerSize);
(asm_.*emit_fn)(addr, index, offset, result, type);
(asm_.*emit_fn)(addr, index, offset, value, result, type);
__ PushRegister(result_type, result);
}
......
......@@ -540,37 +540,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -456,37 +456,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -142,37 +142,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -141,37 +141,38 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAdd");
}
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicSub");
}
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicAnd");
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicOr");
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicXor");
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
bailout(kAtomics, "AtomicExchange");
}
......
......@@ -390,8 +390,15 @@ void LiftoffAssembler::AtomicStore(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
DCHECK(!cache_state()->is_used(value));
LiftoffRegister result, StoreType type) {
DCHECK(!cache_state()->is_used(result));
if (cache_state()->is_used(value)) {
// We cannot overwrite {value}, but the {value} register is changed in the
// code we generate. Therefore we copy {value} to {result} and use the
// {result} register in the code below.
movq(result.gp(), value.gp());
value = result;
}
if (emit_debug_code() && offset_reg != no_reg) {
AssertZeroExtended(offset_reg);
}
......@@ -401,19 +408,25 @@ void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
case StoreType::kI32Store8:
case StoreType::kI64Store8:
xaddb(dst_op, value.gp());
movzxbq(value.gp(), value.gp());
movzxbq(result.gp(), value.gp());
break;
case StoreType::kI32Store16:
case StoreType::kI64Store16:
xaddw(dst_op, value.gp());
movzxwq(value.gp(), value.gp());
movzxwq(result.gp(), value.gp());
break;
case StoreType::kI32Store:
case StoreType::kI64Store32:
xaddl(dst_op, value.gp());
if (value != result) {
movq(result.gp(), value.gp());
}
break;
case StoreType::kI64Store:
xaddq(dst_op, value.gp());
if (value != result) {
movq(result.gp(), value.gp());
}
break;
default:
UNREACHABLE();
......@@ -422,8 +435,15 @@ void LiftoffAssembler::AtomicAdd(Register dst_addr, Register offset_reg,
void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
DCHECK(!cache_state()->is_used(value));
LiftoffRegister result, StoreType type) {
DCHECK(!cache_state()->is_used(result));
if (cache_state()->is_used(value)) {
// We cannot overwrite {value}, but the {value} register is changed in the
// code we generate. Therefore we copy {value} to {result} and use the
// {result} register in the code below.
movq(result.gp(), value.gp());
value = result;
}
if (emit_debug_code() && offset_reg != no_reg) {
AssertZeroExtended(offset_reg);
}
......@@ -434,25 +454,31 @@ void LiftoffAssembler::AtomicSub(Register dst_addr, Register offset_reg,
negb(value.gp());
lock();
xaddb(dst_op, value.gp());
movzxbq(value.gp(), value.gp());
movzxbq(result.gp(), value.gp());
break;
case StoreType::kI32Store16:
case StoreType::kI64Store16:
negw(value.gp());
lock();
xaddw(dst_op, value.gp());
movzxwq(value.gp(), value.gp());
movzxwq(result.gp(), value.gp());
break;
case StoreType::kI32Store:
case StoreType::kI64Store32:
negl(value.gp());
lock();
xaddl(dst_op, value.gp());
if (value != result) {
movq(result.gp(), value.gp());
}
break;
case StoreType::kI64Store:
negq(value.gp());
lock();
xaddq(dst_op, value.gp());
if (value != result) {
movq(result.gp(), value.gp());
}
break;
default:
UNREACHABLE();
......@@ -467,8 +493,8 @@ inline void AtomicBinop(LiftoffAssembler* lasm,
void (Assembler::*opq)(Register, Register),
Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
DCHECK(!__ cache_state()->is_used(value));
LiftoffRegister result, StoreType type) {
DCHECK(!__ cache_state()->is_used(result));
Register value_reg = value.gp();
// The cmpxchg instruction uses rax to store the old value of the
// compare-exchange primitive. Therefore we have to spill the register and
......@@ -535,8 +561,8 @@ inline void AtomicBinop(LiftoffAssembler* lasm,
UNREACHABLE();
}
if (value.gp() != rax) {
__ movq(value.gp(), rax);
if (result.gp() != rax) {
__ movq(result.gp(), rax);
}
}
#undef __
......@@ -544,29 +570,37 @@ inline void AtomicBinop(LiftoffAssembler* lasm,
void LiftoffAssembler::AtomicAnd(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
liftoff::AtomicBinop(this, &Assembler::andl, &Assembler::andq, dst_addr,
offset_reg, offset_imm, value, type);
offset_reg, offset_imm, value, result, type);
}
void LiftoffAssembler::AtomicOr(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
liftoff::AtomicBinop(this, &Assembler::orl, &Assembler::orq, dst_addr,
offset_reg, offset_imm, value, type);
offset_reg, offset_imm, value, result, type);
}
void LiftoffAssembler::AtomicXor(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister value,
StoreType type) {
LiftoffRegister result, StoreType type) {
liftoff::AtomicBinop(this, &Assembler::xorl, &Assembler::xorq, dst_addr,
offset_reg, offset_imm, value, type);
offset_reg, offset_imm, value, result, type);
}
void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
uint32_t offset_imm,
LiftoffRegister value, StoreType type) {
DCHECK(!cache_state()->is_used(value));
LiftoffRegister value,
LiftoffRegister result, StoreType type) {
DCHECK(!cache_state()->is_used(result));
if (cache_state()->is_used(value)) {
// We cannot overwrite {value}, but the {value} register is changed in the
// code we generate. Therefore we copy {value} to {result} and use the
// {result} register in the code below.
movq(result.gp(), value.gp());
value = result;
}
if (emit_debug_code() && offset_reg != no_reg) {
AssertZeroExtended(offset_reg);
}
......@@ -575,19 +609,25 @@ void LiftoffAssembler::AtomicExchange(Register dst_addr, Register offset_reg,
case StoreType::kI32Store8:
case StoreType::kI64Store8:
xchgb(value.gp(), dst_op);
movzxbq(value.gp(), value.gp());
movzxbq(result.gp(), value.gp());
break;
case StoreType::kI32Store16:
case StoreType::kI64Store16:
xchgw(value.gp(), dst_op);
movzxwq(value.gp(), value.gp());
movzxwq(result.gp(), value.gp());
break;
case StoreType::kI32Store:
case StoreType::kI64Store32:
xchgl(value.gp(), dst_op);
if (value != result) {
movq(result.gp(), value.gp());
}
break;
case StoreType::kI64Store:
xchgq(value.gp(), dst_op);
if (value != result) {
movq(result.gp(), value.gp());
}
break;
default:
UNREACHABLE();
......
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