Add acqrel ordering to atomic.fence

CHANGELOG.md

@@ -15,6 +15,11 @@ full changeset diff at the end of each section.
 Current Trunk
 -------------
 
+- Add relaxed atomics support for atomic fences (#8845). Breaks the C and JS
+  APIs; BinaryenAtomicFence / module.atomic.fence now take a memory order param.
+  Use BinaryenMemoryOrderSeqCst() / binaryen.MemoryOrder.seqcst to
+  preserve the original behavior.
+
 v130
 ----
 

scripts/test/generate_atomic_spec_test.py

@@ -38,7 +38,8 @@ class Template:
     bin: bytes = b""
 
 
-templates = [
+atomic_fence_template = Template(op="atomic.fence", value_type=None, args=0, should_drop=False, bin=b"\xfe\x03")
+load_store_acqrel_templates = [
     Template(op="i32.atomic.load", value_type=ValueType.i32, args=1, should_drop=True, bin=b"\xfe\x10"),
     Template(op="i64.atomic.load", value_type=ValueType.i64, args=1, should_drop=True, bin=b"\xfe\x11"),
     Template(op="i32.atomic.load8_u", value_type=ValueType.i64, args=1, should_drop=True, bin=b"\xfe\x12"),
@@ -116,11 +117,20 @@ def all_combinations() -> Iterator[(Template, (int, ValueType), Ordering)]:
     # See the memory section defined in `binary_test`
     memories = [(None, ValueType.i32), (0, ValueType.i32), (1, ValueType.i64)]
 
-    return itertools.product(templates, memories, [None, Ordering.acqrel, Ordering.seqcst])
+    yield from itertools.product(load_store_acqrel_templates, memories, [None, Ordering.acqrel, Ordering.seqcst])
+
+    for ordering in None, Ordering.acqrel, Ordering.seqcst:
+        yield atomic_fence_template, (None, None), ordering
 
 
 def statement(template, mem_idx: int | None, mem_ptr_type: ValueType, ordering: Ordering | None):
     """Return a statement exercising the op in `template` e.g. (i32.atomic.store 1 acqrel (i64.const 42) (i32.const 42))."""
+    if template.op == "atomic.fence":
+        assert mem_idx is None
+        assert mem_ptr_type is None
+
+        return f"(atomic.fence{' ' + ordering.name if ordering is not None else ''})"
+
     memargs = []
     if mem_idx is not None:
         memargs.append(str(mem_idx))
@@ -139,7 +149,7 @@ def statement(template, mem_idx: int | None, mem_ptr_type: ValueType, ordering:
 
 
 def func():
-    """Return a func exercising all ops in `templates`.
+    """Return a func exercising all atomic ops.
 
     e.g.
     (func $test-all-ops
@@ -156,7 +166,7 @@ def func():
 
 
 def text_test():
-    """Return a (module ...) that exercises all ops in `templates`."""
+    """Return a (module ...) that exercises all atomic ops."""
     return f'''(module
   (memory i32 1 1)
   (memory i64 1 1)
@@ -194,6 +204,16 @@ def bin_statement_lines(template: Template, mem_idx: int, mem_ptr_type: ValueTyp
     The entire iterator represents a complete expression using the `template`. e.g.
         (drop (i32.atomic.load (i32.const 42)))
     """
+    if template.op == "atomic.fence":
+        assert mem_idx is None
+        assert mem_ptr_type is None
+
+        assert ordering is not None
+
+        yield template.bin, template.op
+        yield int.to_bytes(ordering.value), f"{ordering.name} memory ordering"
+        return
+
     arg_one_bin = i64_const(0) if mem_ptr_type == ValueType.i64 else i32_const(0)
     yield arg_one_bin, f"({mem_ptr_type.name}.const 0)"
     for _ in range(template.args - 1):
@@ -272,6 +292,11 @@ def binary_func_body():
     statement_bins = []
 
     for template, (mem_idx, mem_ptr_type), ordering in all_combinations():
+        # In the binary encoding for atomic.fence, there's no 'default' without
+        # a memory ordering. Skip this case.
+        if template.op == "atomic.fence" and ordering is None:
+            continue
+
         bin, s = bin_statement(template, mem_idx, mem_ptr_type, ordering)
         statement_bins.append(bin)
         statement_strs.append(s)

scripts/test/relaxed_atomic_execution_tests.py

@@ -79,6 +79,7 @@
   (func (export "i64.atomic.rmw16.cmpxchg_u") (param $addr i32) (param $expected i64) (param $value i64) (result i64) (i64.atomic.rmw16.cmpxchg_u acqrel (local.get $addr) (local.get $expected) (local.get $value)))
   (func (export "i64.atomic.rmw32.cmpxchg_u") (param $addr i32) (param $expected i64) (param $value i64) (result i64) (i64.atomic.rmw32.cmpxchg_u acqrel (local.get $addr) (local.get $expected) (local.get $value)))
 
+  (func (export "atomic.fence") (atomic.fence acqrel))
 )
 
 ;; *.atomic.load*
@@ -390,6 +391,10 @@
 (assert_return (invoke "i64.atomic.rmw32.cmpxchg_u" (i32.const 0) (i64.const 0x11111111) (i64.const 0xcabba6e5cabba6e5)) (i64.const 0x11111111))
 (assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x11111111cabba6e5))
 
+;; atomic.fence
+
+(invoke "atomic.fence")
+
 
 ;; unaligned accesses
 

src/binaryen-c.cpp

@@ -1500,8 +1500,10 @@ BinaryenExpressionRef BinaryenAtomicNotify(BinaryenModuleRef module,
                         0,
                         getMemoryName(module, memoryName)));
 }
-BinaryenExpressionRef BinaryenAtomicFence(BinaryenModuleRef module) {
-  return static_cast<Expression*>(Builder(*(Module*)module).makeAtomicFence());
+BinaryenExpressionRef BinaryenAtomicFence(BinaryenModuleRef module,
+                                          BinaryenMemoryOrder order) {
+  return Builder(*(Module*)module)
+    .makeAtomicFence(static_cast<MemoryOrder>(order));
 }
 BinaryenExpressionRef BinaryenSIMDExtract(BinaryenModuleRef module,
                                           BinaryenOp op,
@@ -3381,15 +3383,18 @@ void BinaryenAtomicNotifySetNotifyCount(BinaryenExpressionRef expr,
     (Expression*)notifyCountExpr;
 }
 // AtomicFence
-uint8_t BinaryenAtomicFenceGetOrder(BinaryenExpressionRef expr) {
+BinaryenMemoryOrder BinaryenAtomicFenceGetOrder(BinaryenExpressionRef expr) {
   auto* expression = (Expression*)expr;
   assert(expression->is<AtomicFence>());
-  return static_cast<AtomicFence*>(expression)->order;
+  return static_cast<BinaryenMemoryOrder>(
+    static_cast<AtomicFence*>(expression)->order);
 }
-void BinaryenAtomicFenceSetOrder(BinaryenExpressionRef expr, uint8_t order) {
+void BinaryenAtomicFenceSetOrder(BinaryenExpressionRef expr,
+                                 BinaryenMemoryOrder order) {
   auto* expression = (Expression*)expr;
   assert(expression->is<AtomicFence>());
-  static_cast<AtomicFence*>(expression)->order = order;
+  static_cast<AtomicFence*>(expression)->order =
+    static_cast<MemoryOrder>(order);
 }
 // SIMDExtract
 BinaryenOp BinaryenSIMDExtractGetOp(BinaryenExpressionRef expr) {

src/binaryen-c.h

@@ -930,7 +930,7 @@ BinaryenAtomicNotify(BinaryenModuleRef module,
                      BinaryenExpressionRef notifyCount,
                      const char* memoryName);
 BINARYEN_API BinaryenExpressionRef
-BinaryenAtomicFence(BinaryenModuleRef module);
+BinaryenAtomicFence(BinaryenModuleRef module, BinaryenMemoryOrder order);
 BINARYEN_API BinaryenExpressionRef
 BinaryenSIMDExtract(BinaryenModuleRef module,
                     BinaryenOp op,
@@ -1961,10 +1961,11 @@ BinaryenAtomicNotifySetNotifyCount(BinaryenExpressionRef expr,
 // AtomicFence
 
 // Gets the order of an `atomic.fence` expression.
-BINARYEN_API uint8_t BinaryenAtomicFenceGetOrder(BinaryenExpressionRef expr);
+BINARYEN_API BinaryenMemoryOrder
+BinaryenAtomicFenceGetOrder(BinaryenExpressionRef expr);
 // Sets the order of an `atomic.fence` expression.
 BINARYEN_API void BinaryenAtomicFenceSetOrder(BinaryenExpressionRef expr,
-                                              uint8_t order);
+                                              BinaryenMemoryOrder order);
 
 // SIMDExtract
 

src/js/binaryen.js-post.js

@@ -2481,8 +2481,8 @@ function wrapModule(module, self = {}) {
   };
 
   self['atomic'] = {
-    'fence'() {
-      return Module['_BinaryenAtomicFence'](module);
+    'fence'(order) {
+      return Module['_BinaryenAtomicFence'](module, order);
     }
   };
 

src/parser/contexts.h

@@ -617,7 +617,7 @@ struct NullInstrParserCtx {
                             MemargT) {
     return Ok{};
   }
-  Result<> makeAtomicFence(Index, const std::vector<Annotation>&) {
+  Result<> makeAtomicFence(Index, const std::vector<Annotation>&, MemoryOrder) {
     return Ok{};
   }
   Result<> makePause(Index, const std::vector<Annotation>&) { return Ok{}; }
@@ -2448,8 +2448,9 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
   }
 
   Result<> makeAtomicFence(Index pos,
-                           const std::vector<Annotation>& annotations) {
-    return withLoc(pos, irBuilder.makeAtomicFence());
+                           const std::vector<Annotation>& annotations,
+                           MemoryOrder order) {
+    return withLoc(pos, irBuilder.makeAtomicFence(order));
   }
 
   Result<> makePause(Index pos, const std::vector<Annotation>& annotations) {

src/parser/parsers.h

@@ -150,7 +150,8 @@ Result<> makeAtomicWait(Ctx&, Index, const std::vector<Annotation>&, Type type);
 template<typename Ctx>
 Result<> makeAtomicNotify(Ctx&, Index, const std::vector<Annotation>&);
 template<typename Ctx>
-Result<> makeAtomicFence(Ctx&, Index, const std::vector<Annotation>&);
+Result<>
+makeAtomicFence(Ctx&, Index, const std::vector<Annotation>&, MemoryOrder);
 template<typename Ctx>
 Result<> makePause(Ctx&, Index, const std::vector<Annotation>&);
 template<typename Ctx>
@@ -1958,12 +1959,14 @@ Result<> makeAtomicNotify(Ctx& ctx,
   CHECK_ERR(arg);
   return ctx.makeAtomicNotify(pos, annotations, mem.getPtr(), *arg);
 }
-
 template<typename Ctx>
 Result<> makeAtomicFence(Ctx& ctx,
                          Index pos,
                          const std::vector<Annotation>& annotations) {
-  return ctx.makeAtomicFence(pos, annotations);
+  auto maybeOrder = maybeMemOrder(ctx);
+  CHECK_ERR(maybeOrder);
+  MemoryOrder order = maybeOrder ? *maybeOrder : MemoryOrder::SeqCst;
+  return ctx.makeAtomicFence(pos, annotations, order);
 }
 
 template<typename Ctx>

src/passes/Print.cpp

@@ -647,7 +647,10 @@ struct PrintExpressionContents
       o << " offset=" << curr->offset;
     }
   }
-  void visitAtomicFence(AtomicFence* curr) { printMedium(o, "atomic.fence"); }
+  void visitAtomicFence(AtomicFence* curr) {
+    printMedium(o, "atomic.fence");
+    printMemoryOrder(curr->order);
+  }
   void visitPause(Pause* curr) { printMedium(o, "pause"); }
   void visitSIMDExtract(SIMDExtract* curr) {
     prepareColor(o);

src/tools/fuzzing/fuzzing.cpp

@@ -5191,7 +5191,7 @@ Expression* TranslateToFuzzReader::makeAtomic(Type type) {
   }
   wasm.memories[0]->shared = true;
   if (type == Type::none) {
-    return builder.makeAtomicFence();
+    return builder.makeAtomicFence(MemoryOrder::SeqCst);
   }
   if (type == Type::i32 && oneIn(2)) {
     if (ATOMIC_WAITS && oneIn(2)) {

src/wasm-builder.h

@@ -428,7 +428,11 @@ class Builder {
     notify->memory = memory;
     return notify;
   }
-  AtomicFence* makeAtomicFence() { return wasm.allocator.alloc<AtomicFence>(); }
+  AtomicFence* makeAtomicFence(MemoryOrder order) {
+    auto* ret = wasm.allocator.alloc<AtomicFence>();
+    ret->order = order;
+    return ret;
+  }
   Pause* makePause() { return wasm.allocator.alloc<Pause>(); }
   Store* makeStore(unsigned bytes,
                    Address offset,

src/wasm-ir-builder.h

@@ -169,7 +169,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
     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();
+  Result<> makeAtomicFence(MemoryOrder order);
   Result<> makePause();
   Result<> makeSIMDExtract(SIMDExtractOp op, uint8_t lane);
   Result<> makeSIMDReplace(SIMDReplaceOp op, uint8_t lane);

src/wasm.h

@@ -1110,10 +1110,7 @@ class AtomicFence : public SpecificExpression<Expression::AtomicFenceId> {
   AtomicFence() = default;
   AtomicFence(MixedArena& allocator) : AtomicFence() {}
 
-  // Current wasm threads only supports sequentially consistent atomics, but
-  // other orderings may be added in the future. This field is reserved for
-  // that, and currently set to 0.
-  uint8_t order = 0;
+  MemoryOrder order = MemoryOrder::SeqCst;
 };
 
 class Pause : public SpecificExpression<Expression::PauseId> {

src/wasm/wasm-binary.cpp

@@ -3991,11 +3991,10 @@ Result<> WasmBinaryReader::readInst() {
           auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
           return builder.makeAtomicNotify(offset, mem);
         }
-        case BinaryConsts::AtomicFence:
-          if (getInt8() != 0) {
-            return Err{"expected 0x00 byte immediate on atomic.fence"};
-          }
-          return builder.makeAtomicFence();
+        case BinaryConsts::AtomicFence: {
+          MemoryOrder order = getMemoryOrder(/*isRMW=*/false);
+          return builder.makeAtomicFence(order);
+        }
         case BinaryConsts::Pause:
           return builder.makePause();
         case BinaryConsts::StructAtomicGet:

src/wasm/wasm-ir-builder.cpp

@@ -1635,8 +1635,9 @@ Result<> IRBuilder::makeAtomicNotify(Address offset, Name mem) {
   return Ok{};
 }
 
-Result<> IRBuilder::makeAtomicFence() {
-  push(builder.makeAtomicFence());
+Result<> IRBuilder::makeAtomicFence(MemoryOrder order) {
+  auto* fence = builder.makeAtomicFence(order);
+  push(fence);
   return Ok{};
 }
 

src/wasm/wasm-stack.cpp

@@ -632,8 +632,21 @@ void BinaryInstWriter::visitAtomicNotify(AtomicNotify* curr) {
 
 void BinaryInstWriter::visitAtomicFence(AtomicFence* curr) {
   o << static_cast<int8_t>(BinaryConsts::AtomicPrefix)
-    << static_cast<int8_t>(BinaryConsts::AtomicFence)
-    << static_cast<int8_t>(curr->order);
+    << static_cast<int8_t>(BinaryConsts::AtomicFence);
+
+  switch (curr->order) {
+    case MemoryOrder::SeqCst: {
+      o << static_cast<int8_t>(0);
+      break;
+    }
+    case MemoryOrder::AcqRel: {
+      o << static_cast<int8_t>(1);
+      break;
+    }
+    case MemoryOrder::Unordered: {
+      WASM_UNREACHABLE("Unexpected memory ordering for atomic.fence");
+    }
+  }
 }
 
 void BinaryInstWriter::visitPause(Pause* curr) {

src/wasm/wasm-validator.cpp

@@ -1446,10 +1446,20 @@ void FunctionValidator::visitAtomicFence(AtomicFence* curr) {
   shouldBeTrue(getModule()->features.hasAtomics(),
                curr,
                "Atomic operations require threads [--enable-threads]");
-  shouldBeTrue(curr->order == 0,
-               curr,
-               "Currently only sequentially consistent atomics are supported, "
-               "so AtomicFence's order should be 0");
+  switch (curr->order) {
+    case MemoryOrder::AcqRel: {
+      shouldBeTrue(getModule()->features.hasRelaxedAtomics(),
+                   curr,
+                   "Acquire/release operations require relaxed atomics "
+                   "[--enable-relaxed-atomics]");
+      break;
+    }
+    case MemoryOrder::SeqCst:
+      break;
+    case MemoryOrder::Unordered:
+      shouldBeTrue(false, curr, "Atomic fence cannot be unordered");
+      break;
+  }
 }
 
 void FunctionValidator::visitPause(Pause* curr) {

test/binaryen.js/atomics.js

@@ -79,7 +79,7 @@ module.addFunction("main", binaryen.none, binaryen.none, [], module.block("", [
     )
   ),
   // fence
-  module.atomic.fence()
+  module.atomic.fence(binaryen.MemoryOrder.seqcst)
 ]));
 
 module.setFeatures(binaryen.Features.Atomics);