From 8a402e4ec4c7260f7e0fb871bcb8dfa7fe822022 Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Fri, 23 Jan 2026 21:10:51 +0000 Subject: [PATCH] Memory order for compoare-exchange --- src/binaryen-c.cpp | 18 +++++++++--------- src/ir/properties.h | 4 ++++ src/parser/contexts.h | 16 +++++++++++----- src/parser/parsers.h | 13 +++++++++++-- src/passes/Print.cpp | 1 + src/tools/fuzzing/fuzzing.cpp | 10 ++++++++-- src/wasm-builder.h | 6 ++++-- src/wasm-delegations-fields.def | 1 + src/wasm-ir-builder.h | 4 ++-- src/wasm.h | 1 + src/wasm/wasm-binary.cpp | 21 ++++++++++++++------- src/wasm/wasm-ir-builder.cpp | 16 ++++++++++------ src/wasm/wasm-stack.cpp | 2 +- src/wasm/wasm-validator.cpp | 22 ++++++++++++++++++++++ 14 files changed, 99 insertions(+), 36 deletions(-) diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index 50d83a5b1fc..825ea59326e 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -1400,15 +1400,15 @@ BinaryenExpressionRef BinaryenAtomicCmpxchg(BinaryenModuleRef module, BinaryenExpressionRef replacement, BinaryenType type, const char* memoryName) { - return static_cast( - Builder(*(Module*)module) - .makeAtomicCmpxchg(bytes, - offset, - (Expression*)ptr, - (Expression*)expected, - (Expression*)replacement, - Type(type), - getMemoryName(module, memoryName))); + return Builder(*(Module*)module) + .makeAtomicCmpxchg(bytes, + offset, + (Expression*)ptr, + (Expression*)expected, + (Expression*)replacement, + Type(type), + getMemoryName(module, memoryName), + MemoryOrder::SeqCst); } BinaryenExpressionRef BinaryenAtomicWait(BinaryenModuleRef module, BinaryenExpressionRef ptr, diff --git a/src/ir/properties.h b/src/ir/properties.h index e28a519e540..ce3fe08c1fa 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -510,6 +510,10 @@ inline MemoryOrder getMemoryOrder(Expression* curr) { if (auto* rmw = curr->dynCast()) { return rmw->order; } + // TODO: this wasn't here before? + if (auto* cmpxchg = curr->dynCast()) { + return cmpxchg->order; + } if (curr->is() || curr->is() || curr->is()) { return MemoryOrder::SeqCst; diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 82083ca82ed..9e9483cbd4c 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -579,8 +579,13 @@ struct NullInstrParserCtx { MemoryOrder) { return Ok{}; } - Result<> makeAtomicCmpxchg( - Index, const std::vector&, Type, int, MemoryIdxT*, MemargT) { + Result<> makeAtomicCmpxchg(Index, + const std::vector&, + Type, + int, + MemoryIdxT*, + MemargT, + MemoryOrder) { return Ok{}; } Result<> makeAtomicWait( @@ -2288,11 +2293,12 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Type type, int bytes, Name* mem, - Memarg memarg) { + Memarg memarg, + MemoryOrder order) { auto m = getMemory(pos, mem); CHECK_ERR(m); - return withLoc(pos, - irBuilder.makeAtomicCmpxchg(bytes, memarg.offset, type, *m)); + return withLoc( + pos, irBuilder.makeAtomicCmpxchg(bytes, memarg.offset, type, *m, order)); } Result<> makeAtomicWait(Index pos, diff --git a/src/parser/parsers.h b/src/parser/parsers.h index b1eb6c71b9d..d606563f1c1 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -1839,10 +1839,19 @@ Result<> makeAtomicCmpxchg(Ctx& ctx, uint8_t bytes) { auto mem = maybeMemidx(ctx); CHECK_ERR(mem); + + auto maybeOrder = maybeMemOrder(ctx); + CHECK_ERR(maybeOrder); + auto arg = memarg(ctx, bytes); CHECK_ERR(arg); - return ctx.makeAtomicCmpxchg( - pos, annotations, type, bytes, mem.getPtr(), *arg); + return ctx.makeAtomicCmpxchg(pos, + annotations, + type, + bytes, + mem.getPtr(), + *arg, + maybeOrder ? *maybeOrder : MemoryOrder::SeqCst); } template diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 6eba429fa44..8a9fb04103f 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -676,6 +676,7 @@ struct PrintExpressionContents } restoreNormalColor(o); printMemoryName(curr->memory, o, wasm); + printMemoryOrder(curr->order); if (curr->offset) { o << " offset=" << curr->offset; } diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 1aeaf9c8576..9f982ce4521 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -4775,8 +4775,14 @@ Expression* TranslateToFuzzReader::makeAtomic(Type type) { } else { auto* expected = make(type); auto* replacement = make(type); - return builder.makeAtomicCmpxchg( - bytes, offset, ptr, expected, replacement, type, wasm.memories[0]->name); + return builder.makeAtomicCmpxchg(bytes, + offset, + ptr, + expected, + replacement, + type, + wasm.memories[0]->name, + MemoryOrder::SeqCst); } } diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 0d09fa661a1..29c9e342729 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -486,7 +486,8 @@ class Builder { Expression* expected, Expression* replacement, Type type, - Name memory) { + Name memory, + MemoryOrder order) { auto* ret = wasm.allocator.alloc(); ret->bytes = bytes; ret->offset = offset; @@ -494,8 +495,9 @@ class Builder { ret->expected = expected; ret->replacement = replacement; ret->type = type; - ret->finalize(); ret->memory = memory; + ret->order = order; + ret->finalize(); return ret; } SIMDExtract* diff --git a/src/wasm-delegations-fields.def b/src/wasm-delegations-fields.def index 143528b769e..895fa8b9276 100644 --- a/src/wasm-delegations-fields.def +++ b/src/wasm-delegations-fields.def @@ -385,6 +385,7 @@ DELEGATE_FIELD_CHILD(AtomicCmpxchg, expected) DELEGATE_FIELD_CHILD(AtomicCmpxchg, ptr) DELEGATE_FIELD_INT(AtomicCmpxchg, bytes) DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, offset) +DELEGATE_FIELD_INT(AtomicCmpxchg, order) DELEGATE_FIELD_NAME_KIND(AtomicCmpxchg, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicCmpxchg) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index 5f06ddd9b39..6ace5587b53 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -166,8 +166,8 @@ class IRBuilder : public UnifiedExpressionVisitor> { Type type, Name mem, MemoryOrder order); - Result<> - makeAtomicCmpxchg(unsigned bytes, Address offset, Type type, Name mem); + Result<> makeAtomicCmpxchg( + unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); Result<> makeAtomicWait(Type type, Address offset, Name mem); Result<> makeAtomicNotify(Address offset, Name mem); Result<> makeAtomicFence(); diff --git a/src/wasm.h b/src/wasm.h index 07f015c0172..f040a1806e5 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -1051,6 +1051,7 @@ class AtomicCmpxchg : public SpecificExpression { Expression* expected; Expression* replacement; Name memory; + MemoryOrder order = MemoryOrder::SeqCst; void finalize(); }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 1aa002f4e03..10297f42367 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -3747,31 +3747,38 @@ Result<> WasmBinaryReader::readInst() { case BinaryConsts::I32AtomicCmpxchg: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(4, offset, Type::i32, mem); + return builder.makeAtomicCmpxchg( + 4, offset, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicCmpxchg8U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(1, offset, Type::i32, mem); + return builder.makeAtomicCmpxchg( + 1, offset, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicCmpxchg16U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(2, offset, Type::i32, mem); + return builder.makeAtomicCmpxchg( + 2, offset, Type::i32, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(8, offset, Type::i64, mem); + return builder.makeAtomicCmpxchg( + 8, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg8U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(1, offset, Type::i64, mem); + return builder.makeAtomicCmpxchg( + 1, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg16U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(2, offset, Type::i64, mem); + return builder.makeAtomicCmpxchg( + 2, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg32U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); - return builder.makeAtomicCmpxchg(4, offset, Type::i64, mem); + return builder.makeAtomicCmpxchg( + 4, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I32AtomicWait: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 0144c8d1ea6..703c33a6bcd 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1518,15 +1518,19 @@ Result<> IRBuilder::makeAtomicRMW(AtomicRMWOp op, return Ok{}; } -Result<> IRBuilder::makeAtomicCmpxchg(unsigned bytes, - Address offset, - Type type, - Name mem) { +Result<> IRBuilder::makeAtomicCmpxchg( + unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) { AtomicCmpxchg curr; curr.memory = mem; CHECK_ERR(ChildPopper{*this}.visitAtomicCmpxchg(&curr, type)); - push(builder.makeAtomicCmpxchg( - bytes, offset, curr.ptr, curr.expected, curr.replacement, type, mem)); + push(builder.makeAtomicCmpxchg(bytes, + offset, + curr.ptr, + curr.expected, + curr.replacement, + type, + mem, + order)); return Ok{}; } diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp index 5d6302596fa..d070f5c3ed3 100644 --- a/src/wasm/wasm-stack.cpp +++ b/src/wasm/wasm-stack.cpp @@ -587,7 +587,7 @@ void BinaryInstWriter::visitAtomicCmpxchg(AtomicCmpxchg* curr) { curr->bytes, curr->offset, curr->memory, - MemoryOrder::SeqCst, + curr->order, /*isRMW=*/true); } diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index f50a51f6f55..4b579865d5d 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -1238,6 +1238,28 @@ void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) { shouldBeTrue(getModule()->features.hasAtomics(), curr, "Atomic operations require threads [--enable-threads]"); + + switch (curr->order) { + case MemoryOrder::AcqRel: { + shouldBeTrue(getModule()->features.hasRelaxedAtomics(), + curr, + "Acquire/release operations require relaxed atomics " + "[--enable-relaxed-atomics]"); + break; + } + // Unordered cmpxchg should be impossible unless there's a bug in the + // parser. + case MemoryOrder::Unordered: { + shouldBeUnequal(curr->order, + MemoryOrder::Unordered, + curr, + "Atomic cmpxchg can't be unordered"); + break; + } + case MemoryOrder::SeqCst: + break; + } + validateMemBytes(curr->bytes, curr->type, curr); shouldBeEqualOrFirstIsUnreachable( curr->ptr->type,