-
Notifications
You must be signed in to change notification settings - Fork 951
feat(Singlepass): add config for strict memory boundary checking #6351
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
base: main
Are you sure you want to change the base?
Changes from all commits
dd85211
b5e3d6c
7659d33
72828dd
dabca26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,9 @@ impl SinglepassCallbacks { | |
| pub struct Singlepass { | ||
| pub(crate) enable_nan_canonicalization: bool, | ||
|
|
||
| /// Forces strict memory boundary checks for every memory access. | ||
| pub(crate) strict_memory_boundary_checks: bool, | ||
|
|
||
| /// The middleware chain. | ||
| pub(crate) middlewares: Vec<Arc<dyn ModuleMiddleware>>, | ||
|
|
||
|
|
@@ -92,12 +95,26 @@ impl Singlepass { | |
| pub fn new() -> Self { | ||
| Self { | ||
| enable_nan_canonicalization: true, | ||
| strict_memory_boundary_checks: false, | ||
| middlewares: vec![], | ||
| callbacks: None, | ||
| num_threads: std::thread::available_parallelism().unwrap_or(NonZero::new(1).unwrap()), | ||
| } | ||
| } | ||
|
|
||
| /// Configure whether Singlepass should always emit strict memory boundary | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't a major behavior change like this use the old behaviour by default? We could switch the default with the next major version?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the change is tightly communicated with our customer, we can do it. Moreover, it's going to align with other Singlepass platforms.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our customers are a limited subset of our users though. In general I'm against significant behaviour changes in minor versions. Guess should have @syrusakbary chime in.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, that’s a good point! Keep in mind that the current RISC-V port of Singlepass is still considered experimental, so we may have some flexibility to make this change. As for minor versions, it’s a bit tricky. On one hand, we aim to do a major release once a year, but in reality, many minor releases introduce breaking changes and arguably should be treated as major versions. This might be worth discussing further. |
||
| /// checks for memory accesses. | ||
| /// | ||
| /// When enabled, Singlepass always requests explicit memory bounds checks | ||
| /// from `op_memory`, even for static memories that would normally rely | ||
| /// on the access violation signal. | ||
| pub fn strict_memory_boundary_checks(&mut self, enable: bool) -> &mut Self { | ||
| self.strict_memory_boundary_checks = enable; | ||
| self | ||
| } | ||
|
|
||
| /// Configure whether Singlepass should canonicalize NaNs during code | ||
| /// generation. | ||
| pub fn canonicalize_nans(&mut self, enable: bool) -> &mut Self { | ||
| self.enable_nan_canonicalization = enable; | ||
| self | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.