From 0521155896fcc8200e02dbc3b9df41451cf1a43a Mon Sep 17 00:00:00 2001 From: Jan-Justin van Tonder Date: Thu, 23 Feb 2023 11:24:56 +0100 Subject: [PATCH] cranelift: Add atomic_rmw to interpreter (#5817) (#5856) As per the linked issue, atomic_rmw was implemented without specific regard for thread safety. Additionally, the relevant filetest (atomic-rmw-little.clif) was enabled and altered to fix an inccorrect call to test function `%atomic_rmw_and_i64` after setting up test function `%atomic_rmw_and_i32`. --- .../filetests/runtests/atomic-rmw-little.clif | 11 ++--- cranelift/interpreter/src/step.rs | 40 +++++++++++++++++-- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/cranelift/filetests/filetests/runtests/atomic-rmw-little.clif b/cranelift/filetests/filetests/runtests/atomic-rmw-little.clif index cfb56f791e..4c09c3f1c3 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 diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index d213722978..c77a5fa43c 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();