pulley: Add simple debugging support

This commit adds a `debug.rs` to Pulley to print out the instruction
being executed and the state of all registers between instructions. This
is turned off by default and does not have a runtime or
environment-based configuration value. Instead changing this requires
changing source code for now. This enables the interpreter loop to
unconditionally use this "debugger" where it'll compile away to nothing
in release/benchmarking situations.

This commit additionally adds this support to the `tail_loop` module and
fixes a few issues there such as it accidentally not being tested in CI
as well as a new `#[cfg]` to use it on stable rust with normal `return`
under the assumption that LLVM is highly likely to do TCO.

.github/workflows/main.yml

@@ -594,9 +594,12 @@ jobs:
     # Check that `pulley-interpreter` compiles with tail calls enabled.  Don't
     # actually run the tests with tail calls enabled, because they are not yet
     # implemented in rustc and cause an ICE.
-    - run: cargo check -p pulley-interpreter
+    - run: cargo check -p pulley-interpreter --all-features
       env:
         RUSTFLAGS: "--cfg pulley_tail_calls"
+    - run: cargo test -p pulley-interpreter --all-features --release
+      env:
+        RUSTFLAGS: "--cfg pulley_assume_llvm_makes_tail_calls"
 
     # Ensure that fuzzers still build.
     #

Cargo.toml

@@ -177,8 +177,13 @@ unused_import_braces = 'warn'
 unused-lifetimes = 'warn'
 unused-macro-rules = 'warn'
 
-# Don't warn about unknown cfg condition in `#[cfg(pulley_tail_calls)]`
-unexpected_cfgs = { level = "warn", check-cfg = ['cfg(pulley_tail_calls)'] }
+# Don't warn about unknown cfgs for pulley
+[workspace.lints.rust.unexpected_cfgs]
+level = "warn"
+check-cfg = [
+  'cfg(pulley_tail_calls)',
+  'cfg(pulley_assume_llvm_makes_tail_calls)',
+]
 
 [workspace.lints.clippy]
 # The default set of lints in Clippy is viewed as "too noisy" right now so

pulley/src/interp.rs

@@ -13,9 +13,10 @@ use core::ops::{Index, IndexMut};
 use core::ptr::NonNull;
 use sptr::Strict;
 
-#[cfg(not(pulley_tail_calls))]
+mod debug;
+#[cfg(all(not(pulley_tail_calls), not(pulley_assume_llvm_makes_tail_calls)))]
 mod match_loop;
-#[cfg(pulley_tail_calls)]
+#[cfg(any(pulley_tail_calls, pulley_assume_llvm_makes_tail_calls))]
 mod tail_loop;
 
 const DEFAULT_STACK_SIZE: usize = 1 << 20; // 1 MiB

pulley/src/interp/debug.rs

@@ -0,0 +1,128 @@
+//! Primitive support for debugging Pulley
+//!
+//! This `Debug` visitor defined in this module is what's actually used as part
+//! of the interpreter loop in Pulley. Due to the code size impact of always
+//! including this and the runtime overhead of always checking a flag this is
+//! enabled/disabled via a `const DEBUG` below. This is currently only really
+//! suitable for one-off debugging while developing locally.
+//!
+//! The hope is that this'll eventually evolve into something more useful, but
+//! for now it's a quick-and-easy way to dump all the instructions that are
+//! executed as well as the values in various registers.
+//!
+//! If debugging is disabled, or in `#[no_std]` mode, then this module should
+//! compile away (e.g. a "zero cost abstraction").
+
+use super::Interpreter;
+use crate::decode::{ExtendedOpVisitor, OpVisitor};
+use crate::imms::*;
+use crate::regs::*;
+use alloc::string::ToString;
+
+// Whether or not debugging is enabled at all.
+const DEBUG: bool = false;
+
+// Whether or not these registers are dumped between each instruction.
+const DEBUG_X_REGS: bool = true;
+const DEBUG_F_REGS: bool = false;
+
+#[cfg(not(feature = "std"))]
+macro_rules! print {
+    ($($t:tt)*) => ({ let _ = format_args!($($t)*); })
+}
+#[cfg(not(feature = "std"))]
+macro_rules! println {
+    () => ();
+    ($($t:tt)*) => ({ let _ = format_args!($($t)*); })
+}
+
+#[repr(transparent)]
+pub(super) struct Debug<'a>(pub Interpreter<'a>);
+
+macro_rules! debug_then_delegate {
+    (
+        $(
+            $( #[$attr:meta] )*
+                $snake_name:ident = $name:ident $( {
+                $(
+                    $( #[$field_attr:meta] )*
+                    $field:ident : $field_ty:ty
+                ),*
+            } )? ;
+        )*
+    ) => {
+        $(
+            $( #[$attr] )*
+            fn $snake_name(&mut self $( $( , $field : $field_ty )* )? ) -> Self::Return {
+                if DEBUG {
+                    println!(
+                        concat!(
+                            stringify!($snake_name),
+                            $(
+                                $(
+                                    " ",
+                                    stringify!($field),
+                                    "={:?}",
+                                )*
+                            )?
+                        ),
+                        $($($field),*)?
+                    );
+                }
+                self.0.$snake_name($( $($field),* )?)
+            }
+        )*
+    }
+}
+
+impl<'a> OpVisitor for Debug<'a> {
+    type BytecodeStream = <Interpreter<'a> as OpVisitor>::BytecodeStream;
+    type Return = <Interpreter<'a> as OpVisitor>::Return;
+
+    fn bytecode(&mut self) -> &mut Self::BytecodeStream {
+        self.0.bytecode()
+    }
+
+    fn before_visit(&mut self) {
+        if !DEBUG {
+            return;
+        }
+        print!("\t{:?}\t", self.bytecode().as_ptr());
+    }
+
+    fn after_visit(&mut self) {
+        if !DEBUG {
+            return;
+        }
+        if DEBUG_X_REGS {
+            for (i, regs) in self.0.state.x_regs.chunks(4).enumerate() {
+                print!("\t\t");
+                for (j, reg) in regs.iter().enumerate() {
+                    let n = i * 4 + j;
+                    let val = reg.get_u64();
+                    let reg = XReg::new(n as u8).unwrap().to_string();
+                    print!(" {reg:>3}={val:#018x}");
+                }
+                println!();
+            }
+        }
+        if DEBUG_F_REGS {
+            for (i, regs) in self.0.state.f_regs.chunks(4).enumerate() {
+                print!("\t\t");
+                for (j, reg) in regs.iter().enumerate() {
+                    let n = i * 4 + j;
+                    let val = reg.get_f64().to_bits();
+                    let reg = FReg::new(n as u8).unwrap().to_string();
+                    print!(" {reg:>3}={val:#018x}");
+                }
+                println!();
+            }
+        }
+    }
+
+    for_each_op!(debug_then_delegate);
+}
+
+impl<'a> ExtendedOpVisitor for Debug<'a> {
+    for_each_extended_op!(debug_then_delegate);
+}

pulley/src/interp/match_loop.rs

@@ -19,8 +19,9 @@ use super::*;
 use crate::decode::unwrap_uninhabited;
 
 impl Interpreter<'_> {
-    pub fn run(mut self) -> Done {
+    pub fn run(self) -> Done {
         let mut decoder = Decoder::new();
+        let mut visitor = debug::Debug(self);
         loop {
             // Here `decode_one` will call the appropriate `OpVisitor` method on
             // `self` via the trait implementation in the module above this.
@@ -29,7 +30,7 @@ impl Interpreter<'_> {
             //
             // This will then continue indefinitely until the bytecode says it's
             // done. Note that only trusted bytecode is interpreted here.
-            match unwrap_uninhabited(decoder.decode_one(&mut self)) {
+            match unwrap_uninhabited(decoder.decode_one(&mut visitor)) {
                 ControlFlow::Continue(()) => {}
                 ControlFlow::Break(done) => break done,
             }

pulley/src/interp/tail_loop.rs

@@ -1,6 +1,35 @@
+//! Support executing the interpreter loop through tail-calls rather than a
+//! source-level `loop`.
+//!
+//! This is an alternative means of executing the interpreter loop of Pulley.
+//! The other method is in `match_loop.rs` which is a `loop` over a `match`
+//! (more-or-less). This file instead transitions between opcodes with
+//! tail-calls.
+//!
+//! At this time this module is more performant but disabled by default. Rust
+//! does not have guaranteed tail call elimination at this time so this is not
+//! a suitable means of writing an interpreter loop. That being said this is
+//! included nonetheless for us to experiment and analyze with.
+//!
+//! There are two methods of using this module:
+//!
+//! * `RUSTFLAGS=--cfg=pulley_assume_llvm_makes_tail_calls` - this compilation
+//!   flag indicates that we should assume that LLVM will optimize to making
+//!   tail calls for things that look like tail calls. Practically this
+//!   probably only happens with `--release` and for popular native
+//!   architectures. It's up to the person compiling to manually
+//!   audit/verify/test that TCO is happening.
+//!
+//! * `RUSTFLAGS=--cfg=pulley_tail_calls` - this compilation flag indicates that
+//!   Rust's nightly-only support for guaranteed tail calls should be used. This
+//!   uses the `become` keyword, for example. At this time this feature of Rust
+//!   is highly experimental and not even complete. It only passes `cargo check`
+//!   at this time but doesn't actually run anywhere.
+
 use super::*;
-use crate::decode::unwrap_uninhabited;
+use crate::decode::{unwrap_uninhabited, ExtendedOpVisitor};
 use crate::opcode::Opcode;
+use crate::ExtendedOpcode;
 
 type Handler = fn(Interpreter<'_>) -> Done;
 
@@ -15,12 +44,20 @@ type Handler = fn(Interpreter<'_>) -> Done;
 /// Macro bodies are just bags of tokens; the body is not parsed until after
 /// they are expanded, and this macro is only expanded when `pulley_tail_calls`
 /// is enabled.
+#[cfg(pulley_tail_calls)]
 macro_rules! tail_call {
     ($e:expr) => {
         become $e
     };
 }
 
+#[cfg(pulley_assume_llvm_makes_tail_calls)]
+macro_rules! tail_call {
+    ($e:expr) => {
+        return $e
+    };
+}
+
 impl Interpreter<'_> {
     pub fn run(mut self) -> Done {
         // Perform a dynamic dispatch through a function pointer indexed by
@@ -101,37 +138,15 @@ macro_rules! define_opcode_handler {
                     crate::decode::operands::$snake_name(i.bytecode())
                 );
             )?
-            match OpVisitor::$snake_name(&mut i, $($($field),*)?) {
-                ControlFlow::Continue(()) => tail_call!(i.run()),
+            let _ = &mut i;
+            let mut debug = debug::Debug(i);
+            match debug.$snake_name($($($field),*)?) {
+                ControlFlow::Continue(()) => tail_call!(debug.0.run()),
                 ControlFlow::Break(done) => done,
             }
         }
     )*};
 }
 
 for_each_op!(define_opcode_handler);
-
-macro_rules! define_extended_opcode_handler {
-    ($(
-        $( #[$attr:meta] )*
-        $snake_name:ident = $name:ident $( {
-            $(
-                $( #[$field_attr:meta] )*
-                $field:ident : $field_ty:ty
-            ),*
-        } )?;
-    )*) => {$(
-        fn $snake_name(mut i: Interpreter<'_>) -> Done {
-            $(
-                let ($($field,)*) = unwrap_uninhabited(
-                    crate::decode::operands::$snake_name(i.bytecode())
-                );
-            )?
-            match ExtendedOpVisitor::$snake_name(&mut i, $($($field),*)?) {
-                ControlFlow::Continue(()) => tail_call!(i.run()),
-                ControlFlow::Break(done) => done,
-            }
-        }
-    )*};
-}
-for_each_extended_op!(define_extended_opcode_handler);
+for_each_extended_op!(define_opcode_handler);