Add relaxed memory ordering for exchange / compare-exchange instructions (#8231)

stevenfontanella · web-flow · commit be62412a0ef7 · 2026-01-29T00:43:25.000Z
Part of #8165 wasm::Properties::getMemoryOrder previously returned Unordered for compare-exchange instructions (unlike other atomic operations which returned SeqCst pre-relaxed atomics). After this PR it returns its real memory ordering.
diff --git a/scripts/test/generate-atomic-spec-test.py b/scripts/test/generate-atomic-spec-test.py
@@ -86,6 +86,20 @@ class Template:
     Template(op="i64.atomic.rmw8.xor_u", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x3e"),
     Template(op="i64.atomic.rmw16.xor_u", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x3f"),
     Template(op="i64.atomic.rmw32.xor_u", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x40"),
+    Template(op="i32.atomic.rmw.xchg", value_type=ValueType.i32, args=2, should_drop=True, bin=b"\xfe\x41"),
+    Template(op="i64.atomic.rmw.xchg", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x42"),
+    Template(op="i32.atomic.rmw8.xchg_u", value_type=ValueType.i32, args=2, should_drop=True, bin=b"\xfe\x43"),
+    Template(op="i32.atomic.rmw16.xchg_u", value_type=ValueType.i32, args=2, should_drop=True, bin=b"\xfe\x44"),
+    Template(op="i64.atomic.rmw8.xchg_u", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x45"),
+    Template(op="i64.atomic.rmw16.xchg_u", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x46"),
+    Template(op="i64.atomic.rmw32.xchg_u", value_type=ValueType.i64, args=2, should_drop=True, bin=b"\xfe\x47"),
+    Template(op="i32.atomic.rmw.cmpxchg", value_type=ValueType.i32, args=3, should_drop=True, bin=b"\xfe\x48"),
+    Template(op="i64.atomic.rmw.cmpxchg", value_type=ValueType.i64, args=3, should_drop=True, bin=b"\xfe\x49"),
+    Template(op="i32.atomic.rmw8.cmpxchg_u", value_type=ValueType.i32, args=3, should_drop=True, bin=b"\xfe\x4a"),
+    Template(op="i32.atomic.rmw16.cmpxchg_u", value_type=ValueType.i32, args=3, should_drop=True, bin=b"\xfe\x4b"),
+    Template(op="i64.atomic.rmw8.cmpxchg_u", value_type=ValueType.i64, args=3, should_drop=True, bin=b"\xfe\x4c"),
+    Template(op="i64.atomic.rmw16.cmpxchg_u", value_type=ValueType.i64, args=3, should_drop=True, bin=b"\xfe\x4d"),
+    Template(op="i64.atomic.rmw32.cmpxchg_u", value_type=ValueType.i64, args=3, should_drop=True, bin=b"\xfe\x4e"),
 ]
 
 
diff --git 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<Expression*>(
-    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
@@ -510,6 +510,9 @@ inline MemoryOrder getMemoryOrder(Expression* curr) {
   if (auto* rmw = curr->dynCast<AtomicRMW>()) {
     return rmw->order;
   }
+  if (auto* cmpxchg = curr->dynCast<AtomicCmpxchg>()) {
+    return cmpxchg->order;
+  }
   if (curr->is<AtomicWait>() || curr->is<AtomicNotify>() ||
       curr->is<AtomicFence>()) {
     return MemoryOrder::SeqCst;
diff --git 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<Annotation>&, Type, int, MemoryIdxT*, MemargT) {
+  Result<> makeAtomicCmpxchg(Index,
+                             const std::vector<Annotation>&,
+                             Type,
+                             int,
+                             MemoryIdxT*,
+                             MemargT,
+                             MemoryOrder) {
     return Ok{};
   }
   Result<> makeAtomicWait(
@@ -2288,11 +2293,12 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, 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
@@ -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<typename Ctx>
diff --git 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
@@ -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
@@ -486,16 +486,18 @@ class Builder {
                                    Expression* expected,
                                    Expression* replacement,
                                    Type type,
-                                   Name memory) {
+                                   Name memory,
+                                   MemoryOrder order) {
     auto* ret = wasm.allocator.alloc<AtomicCmpxchg>();
     ret->bytes = bytes;
     ret->offset = offset;
     ret->ptr = ptr;
     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
@@ -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
@@ -166,8 +166,8 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
                          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
@@ -1051,6 +1051,7 @@ class AtomicCmpxchg : public SpecificExpression<Expression::AtomicCmpxchgId> {
   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
@@ -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
@@ -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
@@ -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
@@ -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,
diff --git a/test/spec/relaxed-atomics.wast b/test/spec/relaxed-atomics.wast

Original file line number	Diff line number	Diff line change
`@@ -676,6 +676,7 @@ struct PrintExpressionContents`
`676`	`676`	`}`
`677`	`677`	`restoreNormalColor(o);`
`678`	`678`	`printMemoryName(curr->memory, o, wasm);`
	`679`	`+ printMemoryOrder(curr->order);`
`679`	`680`	`if (curr->offset) {`
`680`	`681`	`o << " offset=" << curr->offset;`
`681`	`682`	`}`