Repository: bytecodealliance/wasmtime
Pull request #5856 (2023-02-22 21:03:17)

cranelift: Add atomic_rmw to interpreter (#5817)
This PR adds the `atomic_rmw` implementation to the interpreter in accordance with issue #5817 and ensures that the relevant filetest passes.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

- [ x] This has been discussed in issue #..., or if not, please tell us why
  here.
- [ x] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.
- [ x] This PR contains test cases, if meaningful.
- [x ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the [code of
conduct](https://github.com/bytecodealliance/wasmtime/blob/master/CODE_OF_CONDUCT.md).
-->


afonso360: 🚀 Thanks for looking into this! 

> @afonso360, do you want to approve and merge this?

Sure, I'll look at this tomorrow morning!
jan-justin: Of course. I am happy to help.

Thanks @jameysharp !

Patch:
diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs
index d21372297..c77a5fa43 100644
--- a/cranelift/interpreter/src/step.rs
+++ b/cranelift/interpreter/src/step.rs
@@ -7,8 +7,8 @@ use crate::value::{Value, ValueConversionKind, ValueError, ValueResult};
 use cranelift_codegen::data_value::DataValue;
 use cranelift_codegen::ir::condcodes::{FloatCC, IntCC};
 use cranelift_codegen::ir::{
-    types, AbiParam, Block, BlockCall, ExternalName, FuncRef, Function, InstructionData, Opcode,
-    TrapCode, Type, Value as ValueRef,
+    types, AbiParam, AtomicRmwOp, Block, BlockCall, ExternalName, FuncRef, Function,
+    InstructionData, Opcode, TrapCode, Type, Value as ValueRef,
 };
 use log::trace;
 use smallvec::{smallvec, SmallVec};
@@ -1187,7 +1187,41 @@ where
             Value::convert(arg(0)?, ValueConversionKind::ExtractUpper(types::I64))?,
         ]),
         Opcode::Iconcat => assign(Value::concat(arg(0)?, arg(1)?)?),
-        Opcode::AtomicRmw => unimplemented!("AtomicRmw"),
+        Opcode::AtomicRmw => {
+            let op = inst.atomic_rmw_op().unwrap();
+            let val = arg(1)?;
+            let addr = arg(0)?.into_int()? as u64;
+            let loaded = Address::try_from(addr).and_then(|addr| state.checked_load(addr, ctrl_ty));
+            let prev_val = match loaded {
+                Ok(v) => v,
+                Err(e) => return Ok(ControlFlow::Trap(CraneliftTrap::User(memerror_to_trap(e)))),
+            };
+            let prev_val_to_assign = prev_val.clone();
+            let replace = match op {
+                AtomicRmwOp::Xchg => Ok(val),
+                AtomicRmwOp::Add => Value::add(prev_val, val),
+                AtomicRmwOp::Sub => Value::sub(prev_val, val),
+                AtomicRmwOp::And => Value::and(prev_val, val),
+                AtomicRmwOp::Or => Value::or(prev_val, val),
+                AtomicRmwOp::Xor => Value::xor(prev_val, val),
+                AtomicRmwOp::Nand => Value::and(prev_val, val).and_then(V::not),
+                AtomicRmwOp::Smax => Value::max(prev_val, val),
+                AtomicRmwOp::Smin => Value::min(prev_val, val),
+                AtomicRmwOp::Umax => Value::max(
+                    Value::convert(val, ValueConversionKind::ToUnsigned)?,
+                    Value::convert(prev_val, ValueConversionKind::ToUnsigned)?,
+                )
+                .and_then(|v| Value::convert(v, ValueConversionKind::ToSigned)),
+                AtomicRmwOp::Umin => Value::min(
+                    Value::convert(val, ValueConversionKind::ToUnsigned)?,
+                    Value::convert(prev_val, ValueConversionKind::ToUnsigned)?,
+                )
+                .and_then(|v| Value::convert(v, ValueConversionKind::ToSigned)),
+            }?;
+            let stored =
+                Address::try_from(addr).and_then(|addr| state.checked_store(addr, replace));
+            assign_or_memtrap(stored.map(|_| prev_val_to_assign))
+        }
         Opcode::AtomicCas => unimplemented!("AtomicCas"),
         Opcode::AtomicLoad => {
             let load_ty = inst_context.controlling_type().unwrap();


Test patch:
diff --git a/cranelift/filetests/filetests/runtests/atomic-rmw-little.clif b/cranelift/filetests/filetests/runtests/atomic-rmw-little.clif
index cfb56f791..4c09c3f1c 100644
--- a/cranelift/filetests/filetests/runtests/atomic-rmw-little.clif
+++ b/cranelift/filetests/filetests/runtests/atomic-rmw-little.clif
@@ -1,3 +1,4 @@
+test interpret
 test run
 target s390x
 target s390x has_mie2
@@ -116,11 +117,11 @@ block0(v0: i32, v1: i32):
     return v4
 }
 
-; run: %atomic_rmw_and_i64(0, 0) == 0
-; run: %atomic_rmw_and_i64(1, 0) == 0
-; run: %atomic_rmw_and_i64(0, 1) == 0
-; run: %atomic_rmw_and_i64(1, 1) == 1
-; run: %atomic_rmw_and_i64(0xF1FFFEFE, 0xCEFFEFEF) == 0xC0FFEEEE
+; run: %atomic_rmw_and_i32(0, 0) == 0
+; run: %atomic_rmw_and_i32(1, 0) == 0
+; run: %atomic_rmw_and_i32(0, 1) == 0
+; run: %atomic_rmw_and_i32(1, 1) == 1
+; run: %atomic_rmw_and_i32(0xF1FFFEFE, 0xCEFFEFEF) == 0xC0FFEEEE