Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fud2] Rhai Testing #2126

Merged
merged 54 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
c8697c4
[fud2] Make plugins optional
sampsyo Jun 3, 2024
da26bbc
Fix a doc typo
sampsyo Jun 3, 2024
83de16e
Start refactoring plugin loading
sampsyo Jun 3, 2024
28f51b1
Start refactoring builder functions
sampsyo Jun 3, 2024
450260d
Refactor the rest of the functions
sampsyo Jun 3, 2024
9ee8d9a
Gather up a `register` function
sampsyo Jun 3, 2024
4e435f6
Use a fresh engine for each plugin??
sampsyo Jun 3, 2024
6d8f8a5
Run one script at a time
sampsyo Jun 3, 2024
c68814b
Refector script contexts
sampsyo Jun 3, 2024
799ec5c
Refactor runner struct
sampsyo Jun 3, 2024
e3afb77
Further beef up the runner
sampsyo Jun 3, 2024
a6e235e
Further simplify interface
sampsyo Jun 3, 2024
2978ff3
Fix a borrow that lasts too long
sampsyo Jun 3, 2024
f9b97e1
Fix a method name
sampsyo Jun 3, 2024
242372b
Move plugin loading to DriverBuilder?
sampsyo Jun 3, 2024
e08ca9e
translate a bulk of operations into rhai
sgpthomas Jun 3, 2024
11c264e
Merge remote-tracking branch 'origin/main' into fud2-migrate-ops
sgpthomas Jun 4, 2024
648d8b6
use single engine to module resolution caching, enabling imports
sgpthomas Jun 4, 2024
979b34f
switched all plugins to use import system
sgpthomas Jun 4, 2024
c75e8f6
include plugins in debug build
sgpthomas Jun 4, 2024
06838bb
we can now load scripts embedded in the binary
sgpthomas Jun 5, 2024
0c1705c
if a module fails once, don't retry it
sgpthomas Jun 5, 2024
36524bd
sort states and operations in list mode
sgpthomas Jun 5, 2024
503d42b
migrate entirely to plugins
sgpthomas Jun 5, 2024
ea67bb8
gate migration behind a feature
sgpthomas Jun 5, 2024
51f78e9
move sorting to when we run files
sgpthomas Jun 5, 2024
86e2b58
rename plugins to scripts internally
sgpthomas Jun 5, 2024
65edb8b
refactored to have more things live in the resolver
sgpthomas Jun 5, 2024
856e2b0
rename plugins/ to scripts/
sgpthomas Jun 5, 2024
eb086e2
Merge remote-tracking branch 'origin/main' into fud2-migrate-ops
sgpthomas Jun 5, 2024
5e47823
improve error reporting for errors in submodules
sgpthomas Jun 5, 2024
58347a4
slightly improved error messages
sgpthomas Jun 10, 2024
dedbf1f
remove comments from snapshot testing + add plugin testing (feature g…
sgpthomas Jun 10, 2024
ee53a5c
remove comment
sgpthomas Jun 10, 2024
898898e
plugins should only load each setup_fn once
sgpthomas Jun 10, 2024
5cc50e7
test every operation in fud2 with insta
sgpthomas Jun 10, 2024
d606875
fix verilator.rhai to use correct testbench for refmem
sgpthomas Jun 10, 2024
fd167db
use .cloned() instead of .map(|x| x.clone())
sgpthomas Jun 10, 2024
5ba03f0
Merge remote-tracking branch 'origin/main' into fud2-rhai-improved-er…
sgpthomas Jun 11, 2024
34df918
refactor + construct plan directly to exactly specify the op
sgpthomas Jun 13, 2024
5ceb1a5
fix axi.rhai to match axi-wrapped in lib.rs
sgpthomas Jun 13, 2024
f602e83
export setup fns for verilator.rhai and icarus.rhai
sgpthomas Jun 13, 2024
b24fc99
Merge branch 'main' into fud2-rhai-improved-errors
sgpthomas Jun 13, 2024
90658f6
update snapshots for changes in ops
sgpthomas Jun 13, 2024
407735a
remove old snapshot
sgpthomas Jun 13, 2024
2f2ff93
updated rhai scripts to match changes to main
sgpthomas Jun 13, 2024
3df2a83
some Rhai scripting documentation
sgpthomas Jun 17, 2024
9f68ff2
Merge branch 'main' into fud2-rhai-improved-errors
sgpthomas Jun 17, 2024
5a1a488
Merge branch 'main' into fud2-rhai-improved-errors
sgpthomas Jun 18, 2024
55d4e24
disable migrate_to_scripts for now
sgpthomas Jun 18, 2024
1b6162f
update axi.rhai + Rhai API to match Rust API
sgpthomas Jun 18, 2024
548fac5
update snapshot
sgpthomas Jun 18, 2024
d849b9e
fix typo
sgpthomas Jun 19, 2024
345997b
Merge branch 'main' into fud2-rhai-improved-errors
sgpthomas Jun 19, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions fud2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ path = "src/main.rs"

[dev-dependencies]
insta = "1.36.0"
itertools.workspace = true
41 changes: 30 additions & 11 deletions fud2/fud-core/src/script/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
};
use std::{
cell::RefCell,
collections::HashMap,
path::{Path, PathBuf},
rc::Rc,
};
Expand All @@ -20,6 +21,7 @@ struct ScriptContext {
builder: Rc<RefCell<DriverBuilder>>,
path: Rc<PathBuf>,
ast: Rc<rhai::AST>,
setups: Rc<RefCell<HashMap<String, SetupRef>>>,
}

impl ScriptContext {
Expand All @@ -34,17 +36,7 @@ impl ScriptContext {
setups
.into_iter()
.map(|s| match s.clone().try_cast::<rhai::FnPtr>() {
Some(fnptr) => {
let rctx = RhaiSetupCtx {
path: Rc::clone(&self.path),
ast: Rc::new(self.ast.clone_functions_only()),
name: fnptr.fn_name().to_string(),
};
Ok(self.builder.borrow_mut().add_setup(
&format!("{} (plugin)", fnptr.fn_name()),
rctx,
))
}
Some(fnptr) => Ok(self.make_or_get_setupref(fnptr)),
// if we can't cast as a FnPtr, try casting as a SetupRef directly
None => {
s.clone().try_cast::<SetupRef>().ok_or_else(move || {
Expand All @@ -56,13 +48,38 @@ impl ScriptContext {
})
.collect::<RhaiResult<Vec<_>>>()
}

/// Construct a SetupRef for a rhai function. If we already have a SetupRef,
/// return the previously constructed version, otherwise make a new one, cache it
/// and return that.
fn make_or_get_setupref(&self, fnptr: rhai::FnPtr) -> SetupRef {
// if we haven't seen this fnptr before, make a new setup context
// for this function
if !self.setups.borrow().contains_key(fnptr.fn_name()) {
let rctx = RhaiSetupCtx {
path: Rc::clone(&self.path),
ast: Rc::new(self.ast.clone_functions_only()),
name: fnptr.fn_name().to_string(),
};
let setup_ref = self
.builder
.borrow_mut()
.add_setup(&format!("{} (plugin)", fnptr.fn_name()), rctx);
self.setups
.borrow_mut()
.insert(fnptr.fn_name().to_string(), setup_ref);
}

*self.setups.borrow().get(fnptr.fn_name()).unwrap()
}
}

pub struct ScriptRunner {
builder: Rc<RefCell<DriverBuilder>>,
engine: rhai::Engine,
rhai_functions: rhai::AST,
resolver: Option<Resolver>,
setups: Rc<RefCell<HashMap<String, SetupRef>>>,
}

impl ScriptRunner {
Expand All @@ -72,6 +89,7 @@ impl ScriptRunner {
engine: rhai::Engine::new(),
rhai_functions: rhai::AST::empty(),
resolver: Some(Resolver::default()),
setups: Rc::default(),
};
this.reg_state();
this.reg_get_state();
Expand Down Expand Up @@ -184,6 +202,7 @@ impl ScriptRunner {
builder: Rc::clone(&self.builder),
path: Rc::new(path),
ast: Rc::new(self.rhai_functions.clone()),
setups: Rc::clone(&self.setups),
}
}

Expand Down
64 changes: 49 additions & 15 deletions fud2/fud-core/src/script/report.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
use ariadne::{Color, Fmt, Label, Report, ReportKind, Source};
use rhai::EvalAltResult;
use std::{fs, path::Path};
use std::{
fs,
path::{Path, PathBuf},
};

use super::{error::RhaiSystemError, exec_scripts::RhaiResult};

/// A small hack to improve error messages. These are known function names
/// so that we can say that arguments are incorrect when a user calls
/// one of these functions.
const KNOWN_FNS: [&str; 4] = ["state", "op", "get_state", "get_setup"];

pub(super) trait RhaiReport {
fn report_raw<P: AsRef<Path>, S: AsRef<str>>(
&self,
path: P,
len: usize,
header: Option<String>,
msg: S,
);

fn report<P: AsRef<Path>>(&self, path: P) {
self.report_raw(path, 0, "")
self.report_raw(path, 0, None, "")
}
}

Expand All @@ -22,6 +31,7 @@ impl RhaiReport for rhai::Position {
&self,
path: P,
len: usize,
header: Option<String>,
msg: S,
) {
let source = fs::read_to_string(path.as_ref());
Expand All @@ -43,7 +53,9 @@ impl RhaiReport for rhai::Position {
let err_offset = line_offset + (position - 1);

Report::build(ReportKind::Error, name, err_offset)
.with_message("Failed to load plugin")
.with_message(
header.unwrap_or("Failed to load plugin".to_string()),
)
.with_label(
Label::new((name, err_offset..err_offset + len))
.with_message(msg.as_ref().fg(Color::Red)),
Expand All @@ -68,19 +80,25 @@ impl RhaiReport for EvalAltResult {
&self,
path: P,
_len: usize,
header: Option<String>,
_msg: S,
) {
match &self {
EvalAltResult::ErrorVariableNotFound(variable, pos) => {
pos.report_raw(&path, variable.len(), "Undefined variable")
}
EvalAltResult::ErrorVariableNotFound(variable, pos) => pos
.report_raw(
&path,
variable.len(),
header,
"Undefined variable",
),
EvalAltResult::ErrorFunctionNotFound(msg, pos) => {
let (fn_name, args) = msg.split_once(' ').unwrap_or((msg, ""));
pos.report_raw(
&path,
fn_name.len(),
format!("{fn_name} {args}"),
)
let msg = if KNOWN_FNS.contains(&fn_name) {
format!("Invalid arguments. Expected {args}")
} else {
format!("Unknown function: {fn_name} {args}")
};
pos.report_raw(&path, fn_name.len(), header, msg)
}
EvalAltResult::ErrorSystem(msg, err)
if err.is::<RhaiSystemError>() =>
Expand All @@ -91,12 +109,27 @@ impl RhaiReport for EvalAltResult {
} else {
format!("{msg}: {err}")
};
e.position.report_raw(&path, 0, msg)
e.position.report_raw(&path, 0, header, msg)
}
EvalAltResult::ErrorInModule(submod_path, err, _)
if path.as_ref().to_str() == Some(submod_path) =>
{
err.report(submod_path)
}
EvalAltResult::ErrorInModule(path, err, _) => err.report(path),
EvalAltResult::ErrorInModule(submod_path, err, _) => err
.report_raw(
submod_path,
0,
Some(format!(
"Error in submodule {:?} while loading {:?}",
PathBuf::from(submod_path).file_stem().unwrap(),
path.as_ref().file_stem().unwrap()
)),
"",
),
// for errors that we don't have custom processing, just point
// to the beginning of the error, and use the error Display as message
e => e.position().report_raw(&path, 0, format!("{e}")),
e => e.position().report_raw(&path, 0, header, format!("{e}")),
}
}
}
Expand All @@ -106,10 +139,11 @@ impl<T> RhaiReport for RhaiResult<T> {
&self,
path: P,
len: usize,
header: Option<String>,
msg: S,
) {
if let Err(e) = self {
(**e).report_raw(path, len, msg);
(**e).report_raw(path, len, header, msg);
}
}
}
40 changes: 28 additions & 12 deletions fud2/fud-core/src/script/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::{
use itertools::Itertools;
use rhai::EvalAltResult;

use super::{exec_scripts::RhaiResult, report::RhaiReport};
use super::exec_scripts::RhaiResult;

#[derive(Default)]
pub(super) struct Resolver {
Expand All @@ -28,8 +28,8 @@ enum ResolverError {
impl Display for ResolverError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ResolverError::Failed(m) => write!(f, "Loading `{m}` failed."),
ResolverError::Unknown(m) => write!(f, "`{m}` was not found."),
ResolverError::Failed(m) => write!(f, "Loading {m} failed."),
ResolverError::Unknown(m) => write!(f, "{m} was not found."),
}
}
}
Expand All @@ -48,7 +48,6 @@ impl Resolver {
path: PathBuf,
ast: rhai::AST,
) -> rhai::AST {
// let stem = path.file_stem().unwrap().into();
let functions = ast.clone_functions_only();
self.files.push((path, ast));

Expand All @@ -72,7 +71,7 @@ impl Resolver {
.collect()
}

fn resolve_name(&self, name: &str) -> Option<&rhai::AST> {
fn find_ast(&self, name: &str) -> Option<&rhai::AST> {
self.files
.iter()
.find(|(path, _)| {
Expand All @@ -85,6 +84,16 @@ impl Resolver {
.map(|(_, ast)| ast)
}

fn resolve_filename(&self, name: &str) -> Option<&PathBuf> {
self.files
.iter()
.find(|(path, _)| {
Some(name) == path.to_str()
|| Some(name) == path.file_stem().and_then(|os| os.to_str())
})
.map(|(path, _)| path)
}

fn normalize_name(&self, name: &str) -> String {
PathBuf::from(name)
.file_stem()
Expand All @@ -109,7 +118,7 @@ impl Resolver {
let name = self.normalize_name(path);
if self.failed.borrow().contains(&name) {
Err(Box::new(EvalAltResult::ErrorSystem(
"Failed module loading".to_string(),
"".to_string(),
Box::new(ResolverError::Failed(format!("{path:?}"))),
)))
} else {
Expand All @@ -129,9 +138,12 @@ impl rhai::ModuleResolver for Resolver {
engine: &rhai::Engine,
_source: Option<&str>,
name: &str,
_pos: rhai::Position,
pos: rhai::Position,
) -> RhaiResult<Rc<rhai::Module>> {
let path_buf = PathBuf::from(name);
let path_buf = self
.resolve_filename(name)
.cloned()
.unwrap_or(PathBuf::from(name));

// if this path has already failed, don't try loading it again
self.did_fail(name)?;
Expand All @@ -141,7 +153,7 @@ impl rhai::ModuleResolver for Resolver {
Ok(module)
} else {
// otherwise, make a new module, cache it, and return it
self.resolve_name(name)
self.find_ast(name)
.ok_or(ResolverError::Unknown(name.to_string()).into())
.and_then(|ast| {
rhai::Module::eval_ast_as_new(
Expand All @@ -151,10 +163,14 @@ impl rhai::ModuleResolver for Resolver {
)
})
.map(|m| self.insert(name, m))
.inspect_err(|e| {
e.report(&path_buf);
self.add_failed(name)
.map_err(|e| {
Box::new(EvalAltResult::ErrorInModule(
path_buf.as_os_str().to_str().unwrap().to_string(),
e,
pos,
))
})
.inspect_err(|_| self.add_failed(name))
}
}
}
55 changes: 55 additions & 0 deletions fud2/scripts/cocotb-axi.rhai
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import "calyx" as c;

export let cocotb_axi = state("cocotb-axi", ["dat"]);

export let cocotb_setup = cocotb_setup;
fn cocotb_setup(e) {
e.config_var_or("cocotb-makefile-dir", "cocotb.makefile-dir", "$calyx-base/yxi/axi-calyx/cocotb");
// TODO (nate): this is duplicated from the sim_setup above. Can this be shared?
// The input data file. `sim.data` is required.
let data_name = e.config_val("sim.data");
let data_path = e.external_path(data_name);
e.var_("sim_data", data_path);

// Cocotb is wants files relative to the location of the makefile.
// This is annoying to calculate on the fly, so we just copy necessary files to the build directory
e.rule("copy", "cp $in $out");
e.rule("make-cocotb", "make DATA_PATH=$sim_data VERILOG_SOURCE=$in COCOTB_LOG_LEVEL=CRITICAL > $out");
// This cleans up the extra `make` cruft, leaving what is in between `{` and `}.`
e.rule("cleanup-cocotb", "sed -n '/Output:/,/make\\[1\\]/{/Output:/d;/make\\[1\\]/d;p}' $in > $out");
}

op(
"calyx-to-cocotb-axi",
[c::calyx_setup, cocotb_setup],
c::verilog_noverify,
cocotb_axi,
|e, input, output| {
e.build_cmd(
["Makefile"],
"copy",
["$cocotb-makefile-dir/Makefile"],
[],
);
e.build_cmd(
["axi_test.py"],
"copy",
["$cocotb-makefile-dir/axi_test.py"],
[],
);
e.build_cmd(
["run_axi_test.py"],
"copy",
["$cocotb-makefile-dir/run_axi_test.py"],
[],
);
e.build_cmd(
["tmp.dat"],
"make-cocotb",
[input],
["Makefile", "axi_test.py", "run_axi_test.py"],
);

e.build_cmd([output], "cleanup-cocotb", ["tmp.dat"], []);
}
)
Loading
Loading