Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,11 @@ jobs:
run: rustup run nightly-2024-07-10 cargo install --locked --version 0.12.0 cargo-fuzz
- name: Fuzz
run: ./ci/jobs/fuzz.sh
nightly-musttail:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- name: Install latest nightly
run: rustup toolchain install nightly --profile minimal
- name: Build and test with experimental-musttail
run: ./ci/jobs/build-and-test-nightly-musttail.sh
18 changes: 18 additions & 0 deletions ci/jobs/build-and-test-nightly-musttail.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/bash

set -euo pipefail
cd -- "$(dirname -- "${BASH_SOURCE[0]}")"
cd ../..

if [ "$(cat /proc/sys/vm/unprivileged_userfaultfd)" != "1" ]; then
echo "1" | sudo tee /proc/sys/vm/unprivileged_userfaultfd > /dev/null
fi
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here (and it's not needed for interpreter-only tests.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because those are recompiler tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your comment down below to drop some tests. I should be able to remove this line


echo ">> cargo +nightly build (polkavm, experimental-musttail)"
cargo +nightly build -p polkavm --features experimental-musttail

echo ">> cargo +nightly test (polkavm, experimental-musttail)"
cargo +nightly test -p polkavm --features experimental-musttail

echo ">> cargo +nightly test --release (polkavm, experimental-musttail)"
cargo +nightly test --release -p polkavm --features experimental-musttail
Comment thread
kvpanch marked this conversation as resolved.
Outdated
3 changes: 3 additions & 0 deletions crates/polkavm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,6 @@ generic-sandbox = []

# Internal feature for testing. DO NOT USE.
export-internals-for-testing = []

# Musttail (`become`) interpreter dispatch. Requires nightly.
experimental-musttail = []
Comment thread
kvpanch marked this conversation as resolved.
Outdated
62 changes: 52 additions & 10 deletions crates/polkavm/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ use polkavm_common::program::{
use polkavm_common::utils::{align_to_next_page_usize, slice_assume_init_mut, ArcBytes, GasVisitorT};

type Target = u32;

#[cfg(feature = "experimental-musttail")]
type HandlerResult = InterruptKind;
#[cfg(not(feature = "experimental-musttail"))]
type HandlerResult = Target;

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -1711,16 +1715,23 @@ impl InterpretedInstance {
log::trace!("Implicitly resuming at: [{}]", self.next_compiled_offset);
}

let mut offset = self.next_compiled_offset;
loop {
if DEBUG {
self.cycle_counter += 1;
}
#[cfg(feature = "experimental-musttail")]
{
dispatch::<DEBUG>(self, self.next_compiled_offset)
}
#[cfg(not(feature = "experimental-musttail"))]
{
let mut offset = self.next_compiled_offset;
loop {
if DEBUG {
self.cycle_counter += 1;
}

if let Some(handler) = self.compiled_handlers.get(cast(offset).to_usize()) {
offset = handler(self, offset);
} else {
return self.interrupt.clone();
if let Some(handler) = self.compiled_handlers.get(cast(offset).to_usize()) {
offset = handler(self, offset);
} else {
return self.interrupt.clone();
}
}
}
}
Expand Down Expand Up @@ -2504,6 +2515,36 @@ struct Args {

type Handler = for<'a> fn(visitor: &'a mut InterpretedInstance, compiled_offset: Target) -> HandlerResult;

// `become` is parse-time feature-gated, so the musttail dispatcher must live in a separately-loaded file.
#[cfg(feature = "experimental-musttail")]
#[path = "interpreter_musttail.rs"]
mod musttail;

#[cfg(feature = "experimental-musttail")]
use musttail::{dispatch, handler_tail};

#[cfg(not(feature = "experimental-musttail"))]
#[inline(always)]
fn handler_tail<const DEBUG: bool>(_visitor: &mut InterpretedInstance, next_off: Target) -> HandlerResult {
next_off
}

// `become` from the handler itself, so the tail call doesn't depend on
// `handler_tail`/`dispatch` being inlined (which `-O0` may skip).
#[cfg(feature = "experimental-musttail")]
macro_rules! tail_dispatch {
($self:expr, $next_off:expr) => {
become handler_tail::<DEBUG>($self, $next_off)
};
}

#[cfg(not(feature = "experimental-musttail"))]
macro_rules! tail_dispatch {
Comment thread
kvpanch marked this conversation as resolved.
Outdated
($self:expr, $next_off:expr) => {
handler_tail::<DEBUG>($self, $next_off)
};
}

Comment thread
kvpanch marked this conversation as resolved.
Outdated
macro_rules! define_interpreter {
(@define $handler_name:ident $body:block $self:ident $compiled_offset:ident) => {{
impl Args {
Expand Down Expand Up @@ -2949,7 +2990,8 @@ macro_rules! define_interpreter {
#[allow(clippy::needless_lifetimes)]
pub fn $handler_name<'a, $(M: $M_ty,)? $(const $const: $const_ty),+>($self: &'a mut InterpretedInstance, compiled_offset: Target) -> HandlerResult {
let $compiled_offset = compiled_offset;
define_interpreter!(@define $handler_name $body $self $compiled_offset $($arg)*)
let next_off: Target = define_interpreter!(@define $handler_name $body $self $compiled_offset $($arg)*);
tail_dispatch!($self, next_off)
}
)+
}
Expand Down
19 changes: 19 additions & 0 deletions crates/polkavm/src/interpreter_musttail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use super::{HandlerResult, InterpretedInstance, Target};
use polkavm_common::cast::cast;

#[inline(always)]
pub(super) fn handler_tail<const DEBUG: bool>(visitor: &mut InterpretedInstance, next_off: Target) -> HandlerResult {
become dispatch::<DEBUG>(visitor, next_off);
}

#[inline(always)]
pub(super) fn dispatch<const DEBUG: bool>(visitor: &mut InterpretedInstance, off: Target) -> HandlerResult {
if DEBUG {
visitor.cycle_counter += 1;
}
if let Some(&handler) = visitor.compiled_handlers.get(cast(off).to_usize()) {
become handler(visitor, off);
} else {
visitor.interrupt.clone()
}
}
2 changes: 2 additions & 0 deletions crates/polkavm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "experimental-musttail", feature(explicit_tail_calls))]
#![cfg_attr(feature = "experimental-musttail", allow(incomplete_features))]
#![forbid(unused_must_use)]
#![forbid(clippy::missing_safety_doc)]
#![deny(clippy::undocumented_unsafe_blocks)]
Expand Down
Loading