pulley: Fill out remaining 32/64-bit integer operations

This required some extra plumbing to shepherd the precise reason why
signed division trapped to Wasmtime which is done through an extra
`TrapKind` side channel now added.

This then additionally fixes the signed remainder interpreter function
to return 0 on `MIN % -1` which is different from what Rust specifies
(which is to return `None` or panic).

cc bytecodealliance#9783

cranelift/codegen/src/isa/pulley_shared/inst/emit.rs

@@ -490,18 +490,6 @@ fn pulley_emit<P>(
                 RawInst::PushFrame | RawInst::StackAlloc32 { .. } => {
                     sink.add_trap(ir::TrapCode::STACK_OVERFLOW);
                 }
-                RawInst::XDiv32U { .. }
-                | RawInst::XDiv64U { .. }
-                | RawInst::XRem32U { .. }
-                | RawInst::XRem64U { .. } => {
-                    sink.add_trap(ir::TrapCode::INTEGER_DIVISION_BY_ZERO);
-                }
-                RawInst::XDiv32S { .. }
-                | RawInst::XDiv64S { .. }
-                | RawInst::XRem32S { .. }
-                | RawInst::XRem64S { .. } => {
-                    sink.add_trap(ir::TrapCode::INTEGER_OVERFLOW);
-                }
                 _ => {}
             }
             super::generated::emit(raw, sink)

cranelift/codegen/src/isa/pulley_shared/lower.isle

@@ -251,6 +251,21 @@
 (rule (lower (has_type $I32 (clz a))) (pulley_xclz32 a))
 (rule (lower (has_type $I64 (clz a))) (pulley_xclz64 a))
 
+;;;; Rules for `popcnt` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule (lower (has_type $I32 (popcnt a))) (pulley_xpopcnt32 a))
+(rule (lower (has_type $I64 (popcnt a))) (pulley_xpopcnt64 a))
+
+;;;; Rules for `rotl` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule (lower (has_type $I32 (rotl a b))) (pulley_xrotl32 a b))
+(rule (lower (has_type $I64 (rotl a b))) (pulley_xrotl64 a b))
+
+;;;; Rules for `rotr` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule (lower (has_type $I32 (rotr a b))) (pulley_xrotr32 a b))
+(rule (lower (has_type $I64 (rotr a b))) (pulley_xrotr64 a b))
+
 ;;;; Rules for `icmp` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (rule 1 (lower (icmp cc a b @ (value_type $I64)))

crates/wasmtime/src/runtime/vm/interpreter.rs

@@ -3,9 +3,9 @@ use crate::runtime::vm::vmcontext::VMArrayCallNative;
 use crate::runtime::vm::{tls, TrapRegisters, TrapTest, VMContext, VMOpaqueContext};
 use crate::ValRaw;
 use core::ptr::NonNull;
-use pulley_interpreter::interp::{DoneReason, RegType, Val, Vm, XRegVal};
+use pulley_interpreter::interp::{DoneReason, RegType, TrapKind, Val, Vm, XRegVal};
 use pulley_interpreter::{Reg, XReg};
-use wasmtime_environ::{BuiltinFunctionIndex, HostCall};
+use wasmtime_environ::{BuiltinFunctionIndex, HostCall, Trap};
 
 /// Interpreter state stored within a `Store<T>`.
 #[repr(transparent)]
@@ -109,8 +109,8 @@ impl InterpreterRef<'_> {
                     }
                 }
                 // If the VM trapped then process that here and return `false`.
-                DoneReason::Trap(pc) => {
-                    self.trap(pc, setjmp);
+                DoneReason::Trap { pc, kind } => {
+                    self.trap(pc, kind, setjmp);
                     break false;
                 }
             }
@@ -125,30 +125,47 @@ impl InterpreterRef<'_> {
     /// Handles an interpreter trap. This will initialize the trap state stored
     /// in TLS via the `test_if_trap` helper below by reading the pc/fp of the
     /// interpreter and seeing if that's a valid opcode to trap at.
-    fn trap(&mut self, pc: NonNull<u8>, setjmp: Setjmp) {
-        let result = tls::with(|s| {
+    fn trap(&mut self, pc: NonNull<u8>, kind: Option<TrapKind>, setjmp: Setjmp) {
+        let regs = TrapRegisters {
+            pc: pc.as_ptr() as usize,
+            fp: self.0[XReg::fp].get_ptr::<u8>() as usize,
+        };
+        tls::with(|s| {
             let s = s.unwrap();
-            s.test_if_trap(
-                TrapRegisters {
-                    pc: pc.as_ptr() as usize,
-                    fp: self.0[XReg::fp].get_ptr::<u8>() as usize,
-                },
-                None,
-                |_| false,
-            )
-        });
+            match kind {
+                Some(kind) => {
+                    let trap = match kind {
+                        TrapKind::IntegerOverflow => Trap::IntegerOverflow,
+                        TrapKind::DivideByZero => Trap::IntegerDivisionByZero,
+                    };
+                    s.set_jit_trap(regs, None, trap);
+                }
+                None => {
+                    let result = s.test_if_trap(
+                        TrapRegisters {
+                            pc: pc.as_ptr() as usize,
+                            fp: self.0[XReg::fp].get_ptr::<u8>() as usize,
+                        },
+                        None,
+                        |_| false,
+                    );
+                    match result {
+                        // This shouldn't be possible, so this is a fatal error
+                        // if it happens.
+                        TrapTest::NotWasm => {
+                            panic!("pulley trap at {pc:?} without trap code registered")
+                        }
 
-        match result {
-            // This shouldn't be possible, so this is a fatal error if it
-            // happens.
-            TrapTest::NotWasm => panic!("pulley trap at {pc:?} without trap code registered"),
+                        // Not possible with our closure above returning `false`.
+                        TrapTest::HandledByEmbedder => unreachable!(),
 
-            // Not possible with our closure above returning `false`.
-            TrapTest::HandledByEmbedder => unreachable!(),
+                        // Trap was handled, yay! We don't use `jmp_buf`.
+                        TrapTest::Trap { jmp_buf: _ } => {}
+                    }
+                }
+            }
+        });
 
-            // Trap was handled, yay! We don't use `jmp_buf`.
-            TrapTest::Trap { jmp_buf: _ } => {}
-        }
         self.longjmp(setjmp);
     }
 

crates/wast-util/src/lib.rs

@@ -407,7 +407,6 @@ impl WastTest {
                 "misc_testsuite/memory-combos.wast",
                 "misc_testsuite/memory64/simd.wast",
                 "misc_testsuite/memory64/threads.wast",
-                "misc_testsuite/misc_traps.wast",
                 "misc_testsuite/rust_fannkuch.wast",
                 "misc_testsuite/simd/almost-extmul.wast",
                 "misc_testsuite/simd/canonicalize-nan.wast",
@@ -443,10 +442,7 @@ impl WastTest {
                 "spec_testsuite/f64_cmp.wast",
                 "spec_testsuite/float_exprs.wast",
                 "spec_testsuite/float_misc.wast",
-                "spec_testsuite/i32.wast",
-                "spec_testsuite/i64.wast",
                 "spec_testsuite/imports.wast",
-                "spec_testsuite/int_exprs.wast",
                 "spec_testsuite/local_get.wast",
                 "spec_testsuite/local_set.wast",
                 "spec_testsuite/local_tee.wast",

pulley/src/interp.rs

@@ -82,7 +82,7 @@ impl Vm {
 
         match self.call_run(func) {
             DoneReason::ReturnToHost(()) => DoneReason::ReturnToHost(self.call_end(rets)),
-            DoneReason::Trap(pc) => DoneReason::Trap(pc),
+            DoneReason::Trap { pc, kind } => DoneReason::Trap { pc, kind },
             DoneReason::CallIndirectHost { id, resume } => {
                 DoneReason::CallIndirectHost { id, resume }
             }
@@ -684,7 +684,12 @@ mod done {
     /// Reason that the pulley interpreter has ceased execution.
     pub enum DoneReason<T> {
         /// A trap happened at this bytecode instruction.
-        Trap(NonNull<u8>),
+        Trap {
+            /// Which instruction is raising this trap.
+            pc: NonNull<u8>,
+            /// The kind of trap being raised, if known.
+            kind: Option<TrapKind>,
+        },
         /// The `call_indirect_host` instruction was executed.
         CallIndirectHost {
             /// The payload of `call_indirect_host`.
@@ -696,6 +701,13 @@ mod done {
         ReturnToHost(T),
     }
 
+    /// Stored within `DoneReason::Trap`.
+    #[expect(missing_docs, reason = "self-describing variants")]
+    pub enum TrapKind {
+        DivideByZero,
+        IntegerOverflow,
+    }
+
     impl MachineState {
         pub(super) fn debug_assert_done_reason_none(&mut self) {
             debug_assert!(self.done_reason.is_none());
@@ -715,8 +727,13 @@ mod done {
         /// instruction to point to the instruction itself in the trap metadata
         /// returned from the interpreter.
         pub fn done_trap<I: Encode>(&mut self) -> ControlFlow<Done> {
+            self.done_trap_kind::<I>(None)
+        }
+
+        /// Same as `done_trap` but with an explicit `TrapKind`.
+        pub fn done_trap_kind<I: Encode>(&mut self, kind: Option<TrapKind>) -> ControlFlow<Done> {
             let pc = self.current_pc::<I>();
-            self.state.done_reason = Some(DoneReason::Trap(pc));
+            self.state.done_reason = Some(DoneReason::Trap { pc, kind });
             ControlFlow::Break(Done { _priv: () })
         }
 
@@ -738,7 +755,7 @@ mod done {
 }
 
 use done::Done;
-pub use done::DoneReason;
+pub use done::{DoneReason, TrapKind};
 
 struct Interpreter<'a> {
     state: &'a mut MachineState,
@@ -1583,7 +1600,14 @@ impl OpVisitor for Interpreter<'_> {
                 self.state[operands.dst].set_i32(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XDiv32S>(),
+            None => {
+                let kind = if b == 0 {
+                    TrapKind::DivideByZero
+                } else {
+                    TrapKind::IntegerOverflow
+                };
+                self.done_trap_kind::<crate::XDiv32S>(Some(kind))
+            }
         }
     }
 
@@ -1595,7 +1619,14 @@ impl OpVisitor for Interpreter<'_> {
                 self.state[operands.dst].set_i64(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XDiv64S>(),
+            None => {
+                let kind = if b == 0 {
+                    TrapKind::DivideByZero
+                } else {
+                    TrapKind::IntegerOverflow
+                };
+                self.done_trap_kind::<crate::XDiv64S>(Some(kind))
+            }
         }
     }
 
@@ -1607,7 +1638,7 @@ impl OpVisitor for Interpreter<'_> {
                 self.state[operands.dst].set_u32(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XDiv32U>(),
+            None => self.done_trap_kind::<crate::XDiv64U>(Some(TrapKind::DivideByZero)),
         }
     }
 
@@ -1619,31 +1650,41 @@ impl OpVisitor for Interpreter<'_> {
                 self.state[operands.dst].set_u64(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XDiv64U>(),
+            None => self.done_trap_kind::<crate::XDiv64U>(Some(TrapKind::DivideByZero)),
         }
     }
 
     fn xrem32_s(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
         let a = self.state[operands.src1].get_i32();
         let b = self.state[operands.src2].get_i32();
-        match a.checked_rem(b) {
+        let result = if a == i32::MIN && b == -1 {
+            Some(0)
+        } else {
+            a.checked_rem(b)
+        };
+        match result {
             Some(result) => {
                 self.state[operands.dst].set_i32(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XRem32S>(),
+            None => self.done_trap_kind::<crate::XRem32S>(Some(TrapKind::DivideByZero)),
         }
     }
 
     fn xrem64_s(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
         let a = self.state[operands.src1].get_i64();
         let b = self.state[operands.src2].get_i64();
-        match a.checked_rem(b) {
+        let result = if a == i64::MIN && b == -1 {
+            Some(0)
+        } else {
+            a.checked_rem(b)
+        };
+        match result {
             Some(result) => {
                 self.state[operands.dst].set_i64(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XRem64S>(),
+            None => self.done_trap_kind::<crate::XRem64S>(Some(TrapKind::DivideByZero)),
         }
     }
 
@@ -1655,7 +1696,7 @@ impl OpVisitor for Interpreter<'_> {
                 self.state[operands.dst].set_u32(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XRem32U>(),
+            None => self.done_trap_kind::<crate::XRem32U>(Some(TrapKind::DivideByZero)),
         }
     }
 
@@ -1667,7 +1708,7 @@ impl OpVisitor for Interpreter<'_> {
                 self.state[operands.dst].set_u64(result);
                 ControlFlow::Continue(())
             }
-            None => self.done_trap::<crate::XRem64U>(),
+            None => self.done_trap_kind::<crate::XRem64U>(Some(TrapKind::DivideByZero)),
         }
     }
 
@@ -1803,6 +1844,46 @@ impl OpVisitor for Interpreter<'_> {
         ControlFlow::Continue(())
     }
 
+    fn xpopcnt32(&mut self, dst: XReg, src: XReg) -> ControlFlow<Done> {
+        let a = self.state[src].get_u32();
+        self.state[dst].set_u32(a.count_ones());
+        ControlFlow::Continue(())
+    }
+
+    fn xpopcnt64(&mut self, dst: XReg, src: XReg) -> ControlFlow<Done> {
+        let a = self.state[src].get_u64();
+        self.state[dst].set_u64(a.count_ones().into());
+        ControlFlow::Continue(())
+    }
+
+    fn xrotl32(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
+        let a = self.state[operands.src1].get_u32();
+        let b = self.state[operands.src2].get_u32();
+        self.state[operands.dst].set_u32(a.rotate_left(b));
+        ControlFlow::Continue(())
+    }
+
+    fn xrotl64(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
+        let a = self.state[operands.src1].get_u64();
+        let b = self.state[operands.src2].get_u32();
+        self.state[operands.dst].set_u64(a.rotate_left(b));
+        ControlFlow::Continue(())
+    }
+
+    fn xrotr32(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
+        let a = self.state[operands.src1].get_u32();
+        let b = self.state[operands.src2].get_u32();
+        self.state[operands.dst].set_u32(a.rotate_right(b));
+        ControlFlow::Continue(())
+    }
+
+    fn xrotr64(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
+        let a = self.state[operands.src1].get_u64();
+        let b = self.state[operands.src2].get_u32();
+        self.state[operands.dst].set_u64(a.rotate_right(b));
+        ControlFlow::Continue(())
+    }
+
     fn xselect32(
         &mut self,
         dst: XReg,

pulley/src/lib.rs

@@ -200,6 +200,21 @@ macro_rules! for_each_op {
             /// `dst = leading_zeros(src)`
             xclz64 = Xclz64 { dst: XReg, src: XReg };
 
+            /// `low32(dst) = count_ones(low32(src))`
+            xpopcnt32 = Xpopcnt32 { dst: XReg, src: XReg };
+            /// `dst = count_ones(src)`
+            xpopcnt64 = Xpopcnt64 { dst: XReg, src: XReg };
+
+            /// `low32(dst) = rotate_left(low32(src1), low32(src2))`
+            xrotl32 = Xrotl32 { operands: BinaryOperands<XReg> };
+            /// `dst = rotate_left(src1, src2)`
+            xrotl64 = Xrotl64 { operands: BinaryOperands<XReg> };
+
+            /// `low32(dst) = rotate_right(low32(src1), low32(src2))`
+            xrotr32 = Xrotr32 { operands: BinaryOperands<XReg> };
+            /// `dst = rotate_right(src1, src2)`
+            xrotr64 = Xrotr64 { operands: BinaryOperands<XReg> };
+
             /// `low32(dst) = low32(src1) << low5(src2)`
             xshl32 = Xshl32 { operands: BinaryOperands<XReg> };
             /// `low32(dst) = low32(src1) >> low5(src2)`