From 482f36fd50b899ebc2ce08ccd151b52920b118de Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Fri, 16 Jan 2026 23:05:23 +0000 Subject: [PATCH] gemini: add ordering field for rmw --- src/binaryen-c.cpp | 18 ++++++------ src/ir/properties.h | 7 +++-- src/parser/contexts.h | 10 ++++--- src/parser/parsers.h | 2 +- src/tools/fuzzing/fuzzing.cpp | 3 +- src/wasm-builder.h | 6 ++-- src/wasm-delegations-fields.def | 1 + src/wasm-ir-builder.h | 8 ++++-- src/wasm.h | 1 + src/wasm/wasm-binary.cpp | 21 +++++++++----- src/wasm/wasm-ir-builder.cpp | 14 +++++---- src/wasm/wasm-validator.cpp | 50 ++++++++++++++++++++++++++------- 12 files changed, 98 insertions(+), 43 deletions(-) diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index e38e60c24ed..631500f5714 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -1382,15 +1382,15 @@ BinaryenExpressionRef BinaryenAtomicRMW(BinaryenModuleRef module, BinaryenExpressionRef value, BinaryenType type, const char* memoryName) { - return static_cast( - Builder(*(Module*)module) - .makeAtomicRMW(AtomicRMWOp(op), - bytes, - offset, - (Expression*)ptr, - (Expression*)value, - Type(type), - getMemoryName(module, memoryName))); + return Builder(*(Module*)module) + .makeAtomicRMW(AtomicRMWOp(op), + bytes, + offset, + (Expression*)ptr, + (Expression*)value, + Type(type), + Name(memoryName), + MemoryOrder::SeqCst); } BinaryenExpressionRef BinaryenAtomicCmpxchg(BinaryenModuleRef module, BinaryenIndex bytes, diff --git a/src/ir/properties.h b/src/ir/properties.h index e1865bea562..e28a519e540 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -507,8 +507,11 @@ inline MemoryOrder getMemoryOrder(Expression* curr) { if (auto* store = curr->dynCast()) { return store->order; } - if (curr->is() || curr->is() || - curr->is() || curr->is()) { + if (auto* rmw = curr->dynCast()) { + return rmw->order; + } + if (curr->is() || curr->is() || + curr->is()) { return MemoryOrder::SeqCst; } return MemoryOrder::Unordered; diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 1ac3a7c525f..82083ca82ed 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -575,7 +575,8 @@ struct NullInstrParserCtx { Type, int, MemoryIdxT*, - MemargT) { + MemargT, + MemoryOrder) { return Ok{}; } Result<> makeAtomicCmpxchg( @@ -2274,11 +2275,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.makeAtomicRMW(op, bytes, memarg.offset, type, *m)); + return withLoc( + pos, irBuilder.makeAtomicRMW(op, bytes, memarg.offset, type, *m, order)); } Result<> makeAtomicCmpxchg(Index pos, diff --git a/src/parser/parsers.h b/src/parser/parsers.h index 77122643d40..d9ef5d4437d 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -1818,7 +1818,7 @@ Result<> makeAtomicRMW(Ctx& ctx, auto arg = memarg(ctx, bytes); CHECK_ERR(arg); return ctx.makeAtomicRMW( - pos, annotations, op, type, bytes, mem.getPtr(), *arg); + pos, annotations, op, type, bytes, mem.getPtr(), *arg, MemoryOrder::SeqCst); } template diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 22e48ef4ac3..1aeaf9c8576 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -4770,7 +4770,8 @@ Expression* TranslateToFuzzReader::makeAtomic(Type type) { ptr, value, type, - wasm.memories[0]->name); + wasm.memories[0]->name, + MemoryOrder::SeqCst); } else { auto* expected = make(type); auto* replacement = make(type); diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 104b8385ed9..0d09fa661a1 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -466,7 +466,8 @@ class Builder { Expression* ptr, Expression* value, Type type, - Name memory) { + Name memory, + MemoryOrder order) { auto* ret = wasm.allocator.alloc(); ret->op = op; ret->bytes = bytes; @@ -474,8 +475,9 @@ class Builder { ret->ptr = ptr; ret->value = value; ret->type = type; - ret->finalize(); ret->memory = memory; + ret->order = order; + ret->finalize(); return ret; } AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes, diff --git a/src/wasm-delegations-fields.def b/src/wasm-delegations-fields.def index 8cb5df64838..143528b769e 100644 --- a/src/wasm-delegations-fields.def +++ b/src/wasm-delegations-fields.def @@ -375,6 +375,7 @@ DELEGATE_FIELD_CHILD(AtomicRMW, ptr) DELEGATE_FIELD_INT(AtomicRMW, op) DELEGATE_FIELD_INT(AtomicRMW, bytes) DELEGATE_FIELD_ADDRESS(AtomicRMW, offset) +DELEGATE_FIELD_INT(AtomicRMW, order) DELEGATE_FIELD_NAME_KIND(AtomicRMW, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicRMW) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index bbc21afb083..5f06ddd9b39 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -160,8 +160,12 @@ class IRBuilder : public UnifiedExpressionVisitor> { unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); Result<> makeAtomicStore( unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); - Result<> makeAtomicRMW( - AtomicRMWOp op, unsigned bytes, Address offset, Type type, Name mem); + Result<> makeAtomicRMW(AtomicRMWOp op, + unsigned bytes, + Address offset, + Type type, + Name mem, + MemoryOrder order); Result<> makeAtomicCmpxchg(unsigned bytes, Address offset, Type type, Name mem); Result<> makeAtomicWait(Type type, Address offset, Name mem); diff --git a/src/wasm.h b/src/wasm.h index ab6323b5f35..07f015c0172 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -1035,6 +1035,7 @@ class AtomicRMW : public SpecificExpression { Expression* ptr; Expression* value; Name memory; + MemoryOrder order = MemoryOrder::SeqCst; void finalize(); }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 96fd205f908..2c730401242 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -3704,31 +3704,38 @@ Result<> WasmBinaryReader::readInst() { #define RMW(op) \ case BinaryConsts::I32AtomicRMW##op: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 4, offset, Type::i32, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 4, offset, Type::i32, mem, MemoryOrder::SeqCst); \ } \ case BinaryConsts::I32AtomicRMW##op##8U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 1, offset, Type::i32, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 1, offset, Type::i32, mem, MemoryOrder::SeqCst); \ } \ case BinaryConsts::I32AtomicRMW##op##16U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 2, offset, Type::i32, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 2, offset, Type::i32, mem, MemoryOrder::SeqCst); \ } \ case BinaryConsts::I64AtomicRMW##op: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 8, offset, Type::i64, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 8, offset, Type::i64, mem, MemoryOrder::SeqCst); \ } \ case BinaryConsts::I64AtomicRMW##op##8U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 1, offset, Type::i64, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 1, offset, Type::i64, mem, MemoryOrder::SeqCst); \ } \ case BinaryConsts::I64AtomicRMW##op##16U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 2, offset, Type::i64, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 2, offset, Type::i64, mem, MemoryOrder::SeqCst); \ } \ case BinaryConsts::I64AtomicRMW##op##32U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW(RMW##op, 4, offset, Type::i64, mem); \ + return builder.makeAtomicRMW( \ + RMW##op, 4, offset, Type::i64, mem, MemoryOrder::SeqCst); \ } RMW(Add); diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 57a2469dce0..0144c8d1ea6 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1075,7 +1075,7 @@ Result<> IRBuilder::visitEnd() { tryy->name = scope.label; tryy->finalize(tryy->type); push(maybeWrapForLabel(tryy)); - } else if (Try * tryy; + } else if (Try* tryy; (tryy = scope.getCatch()) || (tryy = scope.getCatchAll())) { auto index = scope.getIndex(); setCatchBody(tryy, *expr, index); @@ -1503,14 +1503,18 @@ Result<> IRBuilder::makeAtomicStore( return Ok{}; } -Result<> IRBuilder::makeAtomicRMW( - AtomicRMWOp op, unsigned bytes, Address offset, Type type, Name mem) { +Result<> IRBuilder::makeAtomicRMW(AtomicRMWOp op, + unsigned bytes, + Address offset, + Type type, + Name mem, + MemoryOrder order) { AtomicRMW curr; curr.memory = mem; curr.type = type; CHECK_ERR(visitAtomicRMW(&curr)); - push( - builder.makeAtomicRMW(op, bytes, offset, curr.ptr, curr.value, type, mem)); + push(builder.makeAtomicRMW( + op, bytes, offset, curr.ptr, curr.value, type, mem, order)); return Ok{}; } diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 4e79976579f..6cf82bb43bc 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -1108,11 +1108,17 @@ void FunctionValidator::visitLoad(Load* curr) { curr, "Atomic load should be i32 or i64"); } - if (curr->order == MemoryOrder::AcqRel) { - shouldBeTrue(getModule()->features.hasRelaxedAtomics(), - curr, - "Acquire/release operations require relaxed atomics " - "[--enable-relaxed-atomics]"); + switch (curr->order) { + case MemoryOrder::AcqRel: { + shouldBeTrue(getModule()->features.hasRelaxedAtomics(), + curr, + "Acquire/release operations require relaxed atomics " + "[--enable-relaxed-atomics]"); + break; + } + case MemoryOrder::Unordered: + case MemoryOrder::SeqCst: + break; } if (curr->type == Type::v128) { shouldBeTrue(getModule()->features.hasSIMD(), @@ -1147,11 +1153,17 @@ void FunctionValidator::visitStore(Store* curr) { curr, "Atomic store should be i32 or i64"); } - if (curr->order == MemoryOrder::AcqRel) { - shouldBeTrue(getModule()->features.hasRelaxedAtomics(), - curr, - "Acquire/release operations require relaxed atomics " - "[--enable-relaxed-atomics]"); + switch (curr->order) { + case MemoryOrder::AcqRel: { + shouldBeTrue(getModule()->features.hasRelaxedAtomics(), + curr, + "Acquire/release operations require relaxed atomics " + "[--enable-relaxed-atomics]"); + break; + } + case MemoryOrder::Unordered: + case MemoryOrder::SeqCst: + break; } if (curr->valueType == Type::v128) { shouldBeTrue(getModule()->features.hasSIMD(), @@ -1185,6 +1197,24 @@ void FunctionValidator::visitAtomicRMW(AtomicRMW* 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 RMW should be impossible unless there's a bug in the code. + case MemoryOrder::Unordered: { + WASM_UNREACHABLE("Atomic RMW can't be unordered"); + break; + } + case MemoryOrder::SeqCst: + break; + } + validateMemBytes(curr->bytes, curr->type, curr); shouldBeEqualOrFirstIsUnreachable( curr->ptr->type,