diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index 7f04231a2c9..7a1829befbf 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -1368,7 +1368,8 @@ BinaryenExpressionRef BinaryenAtomicStore(BinaryenModuleRef module, (Expression*)ptr, (Expression*)value, Type(type), - getMemoryName(module, memoryName))); + getMemoryName(module, memoryName), + MemoryOrder::SeqCst)); } BinaryenExpressionRef BinaryenAtomicRMW(BinaryenModuleRef module, BinaryenOp op, diff --git a/src/parser/contexts.h b/src/parser/contexts.h index df1e16ad3fb..1ac3a7c525f 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -565,7 +565,8 @@ struct NullInstrParserCtx { int, bool, MemoryIdxT*, - MemargT) { + MemargT, + MemoryOrder) { return Ok{}; } Result<> makeAtomicRMW(Index, @@ -2255,12 +2256,13 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { int bytes, bool isAtomic, Name* mem, - Memarg memarg) { + Memarg memarg, + MemoryOrder order) { auto m = getMemory(pos, mem); CHECK_ERR(m); if (isAtomic) { - return withLoc(pos, - irBuilder.makeAtomicStore(bytes, memarg.offset, type, *m)); + return withLoc( + pos, irBuilder.makeAtomicStore(bytes, memarg.offset, type, *m, order)); } return withLoc( pos, irBuilder.makeStore(bytes, memarg.offset, memarg.align, type, *m)); diff --git a/src/parser/parsers.h b/src/parser/parsers.h index bfea96f968d..77122643d40 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -850,17 +850,6 @@ Result memtypeContinued(Ctx& ctx, Type addressType) { return ctx.makeMemType(addressType, *limits, shared); } -// memorder ::= '' | 'seqcst' | 'acqrel' -template Result memorder(Ctx& ctx) { - if (ctx.in.takeKeyword("seqcst"sv)) { - return MemoryOrder::SeqCst; - } - if (ctx.in.takeKeyword("acqrel"sv)) { - return MemoryOrder::AcqRel; - } - return MemoryOrder::SeqCst; -} - // memorder ::= 'seqcst' | 'acqrel' template MaybeResult maybeMemOrder(Ctx& ctx) { if (ctx.in.takeKeyword("seqcst"sv)) { @@ -873,6 +862,13 @@ template MaybeResult maybeMemOrder(Ctx& ctx) { return {}; } +// memorder ::= '' | 'seqcst' | 'acqrel' +template Result memorder(Ctx& ctx) { + auto order = maybeMemOrder(ctx); + CHECK_ERR(order); + return order ? *order : MemoryOrder::SeqCst; +} + // tabletype ::= (limits32 | 'i32' limits32 | 'i64' limit64) reftype template Result tabletype(Ctx& ctx) { Type addressType = Type::i32; @@ -1755,7 +1751,8 @@ Result<> makeLoad(Ctx& ctx, CHECK_ERR(mem); // We could only parse this when `isAtomic`, but this way gives a clearer - // error since `memIdx` can never be mistaken for a `memOrder`. + // error when a memorder is given for non-atomic operations + // since the next token can never be mistaken for a `memOrder`. auto maybeOrder = maybeMemOrder(ctx); CHECK_ERR(maybeOrder); @@ -1787,10 +1784,26 @@ Result<> makeStore(Ctx& ctx, bool isAtomic) { auto mem = maybeMemidx(ctx); CHECK_ERR(mem); + + auto maybeOrder = maybeMemOrder(ctx); + CHECK_ERR(maybeOrder); + + if (maybeOrder && !isAtomic) { + return Err{"Memory ordering can only be provided for atomic stores."}; + } + auto arg = memarg(ctx, bytes); CHECK_ERR(arg); - return ctx.makeStore( - pos, annotations, type, bytes, isAtomic, mem.getPtr(), *arg); + return ctx.makeStore(pos, + annotations, + type, + bytes, + isAtomic, + mem.getPtr(), + *arg, + maybeOrder ? *maybeOrder + : isAtomic ? MemoryOrder::SeqCst + : MemoryOrder::Unordered); } template diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 96b5c84edab..8fccb8ade81 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -605,6 +605,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/wasm-split/instrumenter.cpp b/src/tools/wasm-split/instrumenter.cpp index 085715ce246..92861a8729a 100644 --- a/src/tools/wasm-split/instrumenter.cpp +++ b/src/tools/wasm-split/instrumenter.cpp @@ -151,7 +151,8 @@ void Instrumenter::instrumentFuncs() { builder.makeConstPtr(0, Type::i32), builder.makeConst(uint32_t(1)), Type::i32, - memoryName), + memoryName, + MemoryOrder::SeqCst), func->body, func->body->type); ++funcIdx; diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 6878e0dff7b..104b8385ed9 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -451,9 +451,13 @@ class Builder { Expression* ptr, Expression* value, Type type, - Name memory) { + Name memory, + MemoryOrder order) { + assert(order != MemoryOrder::Unordered && + "Atomic stores can't be unordered"); + Store* store = makeStore(bytes, offset, bytes, ptr, value, type, memory); - store->order = MemoryOrder::SeqCst; + store->order = order; return store; } AtomicRMW* makeAtomicRMW(AtomicRMWOp op, diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index a3845b385c6..bbc21afb083 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -158,7 +158,8 @@ class IRBuilder : public UnifiedExpressionVisitor> { unsigned bytes, Address offset, unsigned align, Type type, Name mem); Result<> makeAtomicLoad( unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); - Result<> makeAtomicStore(unsigned bytes, Address offset, Type type, Name mem); + 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<> diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index d24f7da423f..a19727e7acf 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -3655,31 +3655,38 @@ Result<> WasmBinaryReader::readInst() { } case BinaryConsts::I32AtomicStore8: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(1, offset, Type::i32, mem); + return builder.makeAtomicStore( + 1, offset, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicStore16: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(2, offset, Type::i32, mem); + return builder.makeAtomicStore( + 2, offset, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicStore: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(4, offset, Type::i32, mem); + return builder.makeAtomicStore( + 4, offset, Type::i32, mem, memoryOrder); } case BinaryConsts::I64AtomicStore8: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(1, offset, Type::i64, mem); + return builder.makeAtomicStore( + 1, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicStore16: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(2, offset, Type::i64, mem); + return builder.makeAtomicStore( + 2, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicStore32: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(4, offset, Type::i64, mem); + return builder.makeAtomicStore( + 4, offset, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicStore: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicStore(8, offset, Type::i64, mem); + return builder.makeAtomicStore( + 8, offset, Type::i64, mem, memoryOrder); } #define RMW(op) \ diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 200a9e0d107..57a2469dce0 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1492,15 +1492,14 @@ Result<> IRBuilder::makeAtomicLoad( return Ok{}; } -Result<> IRBuilder::makeAtomicStore(unsigned bytes, - Address offset, - Type type, - Name mem) { +Result<> IRBuilder::makeAtomicStore( + unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) { Store curr; curr.memory = mem; curr.valueType = type; CHECK_ERR(visitStore(&curr)); - push(builder.makeAtomicStore(bytes, offset, curr.ptr, curr.value, type, mem)); + push(builder.makeAtomicStore( + bytes, offset, curr.ptr, curr.value, type, mem, order)); return Ok{}; } diff --git a/test/spec/relaxed-atomics.wast b/test/spec/relaxed-atomics.wast index d76a2f62b00..b2ac20ef30e 100644 --- a/test/spec/relaxed-atomics.wast +++ b/test/spec/relaxed-atomics.wast @@ -2,11 +2,13 @@ (memory $0 23 256 shared) (func $acqrel (result i32) + (i32.atomic.store acqrel (i32.const 0) (i32.const 0)) (i32.atomic.load acqrel (i32.const 1) ) ) (func $seqcst (result i32) + (i32.atomic.store 0 seqcst (i32.const 0) (i32.const 0)) ;; seqcst may be omitted for atomic loads, it's the default (drop (i32.atomic.load seqcst (i32.const 1) @@ -30,6 +32,39 @@ "Memory ordering can only be provided for atomic loads." ) +;; Parses acquire-release immediate +;; (module +;; (memory $0 23 256 shared) +;; (func $acqrel +;; (i32.atomic.store (i32.const 0) (i32.const 0)) +;; ) +;; ) +(module binary + "\00asm\01\00\00\00" + "\01\04\01\60\00\00\03\02\01\00\05\05\01\03\17\80\02" ;; other sections + "\0a\0d\01" ;; code section + "\0b\00" ;; func $acqrel + "\41\00\41\00" ;; (i32.const 0) (i32.const 0) + "\fe\17" ;; i32.atomic.store + "\22" ;; 2 | (1<<5): Alignment of 2 (32-bit store), with bit 5 set indicating that then next byte is a memory ordering + "\01" ;; acqrel ordering + "\00" ;; offset + "\0b" ;; end +) + +(module binary + "\00asm\01\00\00\00" + "\01\04\01\60\00\00\03\02\01\00\05\05\01\03\17\80\02" ;; other sections + "\0a\0d\01" ;; code section + "\0b\00" ;; func $acqrel + "\41\00\41\00" ;; (i32.const 0) (i32.const 0) + "\fe\17" ;; i32.atomic.store + "\22" ;; 2 | (1<<5): Alignment of 2 (32-bit store), with bit 5 set indicating that then next byte is a memory ordering + "\00" ;; seqcst ordering + "\00" ;; offset + "\0b" ;; end +) + ;; Parses acquire-release immediate ;; Equivalent to ;; (module