Skip to content

Commit fa45d6a

Browse files
committed
deps: V8: cherry-pick 93b2105fbe44
Original commit message: [wasm][ia32][liftoff] Implement AtomicCompareExchange As there are not enough registers on ia32 to execute the platform- independent code, the CL also adds ia32-specific code to liftoff-compiler.cc. For this we first retrieve the memory index from the stack, do a bounds check, and calculate the final address. Only afterwards we pop all other values from the stack and pass them to the platform-dependent code. [email protected] Bug: v8:10108 Change-Id: I741266a9523c8b5c46acc0b29817fd143a75752e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2316305 Reviewed-by: Clemens Backes <[email protected]> Commit-Queue: Andreas Haas <[email protected]> Cr-Commit-Position: refs/heads/master@{#69047} Refs: v8/v8@93b2105 PR-URL: #38275 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
1 parent c5fe3a2 commit fa45d6a

File tree

3 files changed

+150
-5
lines changed

3 files changed

+150
-5
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.53',
39+
'v8_embedder_string': '-node.54',
4040

4141
##### V8 defaults for Node.js #####
4242

deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h

+117-1
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,123 @@ void LiftoffAssembler::AtomicCompareExchange(
532532
Register dst_addr, Register offset_reg, uint32_t offset_imm,
533533
LiftoffRegister expected, LiftoffRegister new_value, LiftoffRegister result,
534534
StoreType type) {
535-
bailout(kAtomics, "AtomicCompareExchange");
535+
// We expect that the offset has already been added to {dst_addr}, and no
536+
// {offset_reg} is provided. This is to save registers.
537+
DCHECK_EQ(offset_reg, no_reg);
538+
539+
DCHECK_EQ(result, expected);
540+
541+
if (type.value() != StoreType::kI64Store) {
542+
bool is_64_bit_op = type.value_type() == kWasmI64;
543+
544+
Register value_reg = is_64_bit_op ? new_value.low_gp() : new_value.gp();
545+
Register expected_reg = is_64_bit_op ? expected.low_gp() : expected.gp();
546+
Register result_reg = expected_reg;
547+
548+
bool is_byte_store = type.size() == 1;
549+
LiftoffRegList pinned =
550+
LiftoffRegList::ForRegs(dst_addr, value_reg, expected_reg);
551+
552+
// Ensure that {value_reg} is a valid register.
553+
if (is_byte_store && !liftoff::kByteRegs.has(value_reg)) {
554+
Register safe_value_reg =
555+
pinned.set(GetUnusedRegister(liftoff::kByteRegs, pinned)).gp();
556+
mov(safe_value_reg, value_reg);
557+
value_reg = safe_value_reg;
558+
pinned.clear(LiftoffRegister(value_reg));
559+
}
560+
561+
// The cmpxchg instruction uses eax to store the old value of the
562+
// compare-exchange primitive. Therefore we have to spill the register and
563+
// move any use to another register.
564+
ClearRegister(eax, {&dst_addr, &value_reg}, pinned);
565+
if (expected_reg != eax) {
566+
mov(eax, expected_reg);
567+
}
568+
569+
Operand dst_op = Operand(dst_addr, offset_imm);
570+
571+
lock();
572+
switch (type.value()) {
573+
case StoreType::kI32Store8:
574+
case StoreType::kI64Store8: {
575+
cmpxchg_b(dst_op, value_reg);
576+
movzx_b(result_reg, eax);
577+
break;
578+
}
579+
case StoreType::kI32Store16:
580+
case StoreType::kI64Store16: {
581+
cmpxchg_w(dst_op, value_reg);
582+
movzx_w(result_reg, eax);
583+
break;
584+
}
585+
case StoreType::kI32Store:
586+
case StoreType::kI64Store32: {
587+
cmpxchg(dst_op, value_reg);
588+
if (result_reg != eax) {
589+
mov(result_reg, eax);
590+
}
591+
break;
592+
}
593+
default:
594+
UNREACHABLE();
595+
}
596+
if (is_64_bit_op) {
597+
xor_(result.high_gp(), result.high_gp());
598+
}
599+
return;
600+
}
601+
602+
// The following code handles kExprI64AtomicCompareExchange.
603+
604+
// We need {ebx} here, which is the root register. The root register it
605+
// needs special treatment. As we use {ebx} directly in the code below, we
606+
// have to make sure here that the root register is actually {ebx}.
607+
static_assert(kRootRegister == ebx,
608+
"The following code assumes that kRootRegister == ebx");
609+
push(kRootRegister);
610+
611+
// The compare-exchange instruction uses registers as follows:
612+
// old-value = EDX:EAX; new-value = ECX:EBX.
613+
Register expected_hi = edx;
614+
Register expected_lo = eax;
615+
Register new_hi = ecx;
616+
Register new_lo = ebx;
617+
// The address needs a separate registers that does not alias with the
618+
// ones above.
619+
Register address = esi;
620+
621+
// Spill all these registers if they are still holding other values.
622+
liftoff::SpillRegisters(this, expected_hi, expected_lo, new_hi, address);
623+
624+
// We have to set new_lo specially, because it's the root register. We do it
625+
// before setting all other registers so that the original value does not get
626+
// overwritten.
627+
mov(new_lo, new_value.low_gp());
628+
629+
// Move all other values into the right register.
630+
{
631+
LiftoffAssembler::ParallelRegisterMoveTuple reg_moves[]{
632+
{LiftoffRegister(address), LiftoffRegister(dst_addr), kWasmI32},
633+
{LiftoffRegister::ForPair(expected_lo, expected_hi), expected, kWasmI64},
634+
{LiftoffRegister(new_hi), new_value.high(), kWasmI32}};
635+
ParallelRegisterMove(ArrayVector(reg_moves));
636+
};
637+
638+
Operand dst_op = Operand(address, offset_imm);
639+
640+
lock();
641+
cmpxchg8b(dst_op);
642+
643+
// Restore the root register, and we are done.
644+
pop(kRootRegister);
645+
646+
// Move the result into the correct registers.
647+
{
648+
LiftoffAssembler::ParallelRegisterMoveTuple reg_moves[]{
649+
{result, LiftoffRegister::ForPair(expected_lo, expected_hi), kWasmI64}};
650+
ParallelRegisterMove(ArrayVector(reg_moves));
651+
}
536652
}
537653

538654
void LiftoffAssembler::AtomicFence() { mfence(); }

deps/v8/src/wasm/baseline/liftoff-compiler.cc

+32-3
Original file line numberDiff line numberDiff line change
@@ -2879,9 +2879,38 @@ class LiftoffCompiler {
28792879
void AtomicCompareExchange(FullDecoder* decoder, StoreType type,
28802880
const MemoryAccessImmediate<validate>& imm) {
28812881
#ifdef V8_TARGET_ARCH_IA32
2882-
// With the current implementation we do not have enough registers on ia32
2883-
// to even get to the platform-specific code. Therefore we bailout early.
2884-
unsupported(decoder, kAtomics, "AtomicCompareExchange");
2882+
// On ia32 we don't have enough registers to first pop all the values off
2883+
// the stack and then start with the code generation. Instead we do the
2884+
// complete address calculation first, so that the address only needs a
2885+
// single register. Afterwards we load all remaining values into the
2886+
// other registers.
2887+
LiftoffRegList pinned;
2888+
Register index_reg = pinned.set(__ PeekToRegister(2, pinned)).gp();
2889+
if (BoundsCheckMem(decoder, type.size(), imm.offset, index_reg, pinned,
2890+
kDoForceCheck)) {
2891+
return;
2892+
}
2893+
AlignmentCheckMem(decoder, type.size(), imm.offset, index_reg, pinned);
2894+
2895+
uint32_t offset = imm.offset;
2896+
index_reg = AddMemoryMasking(index_reg, &offset, &pinned);
2897+
Register addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp();
2898+
LOAD_INSTANCE_FIELD(addr, MemoryStart, kSystemPointerSize);
2899+
__ emit_i32_add(addr, addr, index_reg);
2900+
pinned.clear(LiftoffRegister(index_reg));
2901+
LiftoffRegister new_value = pinned.set(__ PopToRegister(pinned));
2902+
LiftoffRegister expected = pinned.set(__ PopToRegister(pinned));
2903+
2904+
// Pop the index from the stack.
2905+
__ cache_state()->stack_state.pop_back(1);
2906+
2907+
LiftoffRegister result = expected;
2908+
2909+
// We already added the index to addr, so we can just pass no_reg to the
2910+
// assembler now.
2911+
__ AtomicCompareExchange(addr, no_reg, offset, expected, new_value, result,
2912+
type);
2913+
__ PushRegister(type.value_type(), result);
28852914
return;
28862915
#else
28872916
ValueType result_type = type.value_type();

0 commit comments

Comments
 (0)