-
Notifications
You must be signed in to change notification settings - Fork 824
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
Add AArch64 support for singlepass. #713
Conversation
285d7e9
to
e6c6705
Compare
e6c6705
to
4210715
Compare
9ab3da3
to
b57aba4
Compare
bors try |
tryBuild failed |
bors try |
tryBuild succeeded |
# Conflicts: # Makefile
… feature/singlepass-aarch64
bors try |
tryBuild succeeded |
bors r+ |
713: Add AArch64 support for singlepass. r=syrusakbary a=losfair This PR includes: - Support for AArch64 (ARM64) in Singlepass backend. Implemented with a combination of x86_64 instruction translation and native code generation. - State tracing and backtraces on AArch64/Singlepass. (Tiering is not implemented for this pair because there's no other backend supporting AArch64 yet) - Debugging tools: `BlockTrace` middleware, and support for reading states of previous WebAssembly stack frames from middlewares. Co-authored-by: losfair <[email protected]>
Build succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me! Though a PR this long is very hard to review -- I ended up skipping most of the code generation and translation changes 🤷♂ ; it'd be nice if changes like this could be merged in with 10-20 PRs of reasonable size
pub fn setjmp(env: *mut c_void) -> i32; | ||
/// libc longjmp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We depend on libc
in runtime-core; is there a reason we aren't using the libc version?
const TRAP_STACK_SIZE: usize = 1048576; // 1MB | ||
|
||
const SETJMP_BUFFER_LEN: usize = 27; | ||
const SETJMP_BUFFER_LEN: usize = 128; | ||
type SetJmpBuffer = [i32; SETJMP_BUFFER_LEN]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repinging on this
|
||
FaultInfo { | ||
faulting_addr: si_addr as usize as _, | ||
ip: gregs[REG_RIP as usize] as _, | ||
ip: std::mem::transmute::<&mut i64, &'static Cell<usize>>(&mut gregs[REG_RIP as usize]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can probably cause undefined behavior but it's pretty subtle. The fact that a normal type in Rust exists that is being accessed with Cell
or UnsafeCell
is unsound. Cell
by way of UnsafeCell
is the only way to have aliased mutability in Rust without undefined behavior. This code allows aliased mutability where one side may be a Rust type without Cell
or UnsafeCell
. I believe that is undefined behavior.
&mut self.machine, | ||
tmp_in, | ||
-1.0, | ||
18446744073709551616.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛳️
u64::max_value() as f64
(if this is the same, then I think it's more readable)
@@ -397,7 +403,7 @@ fn execute_wasm(options: &Run) -> Result<(), String> { | |||
.map_err(|e| format!("Can't convert from wast to wasm: {:?}", e))?; | |||
} | |||
|
|||
let compiler: Box<dyn Compiler> = match get_compiler_by_backend(options.backend) { | |||
let compiler: Box<dyn Compiler> = match get_compiler_by_backend(options.backend, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repinging on this
1068: Various cleanups for the singlepass backend on AArch64. r=losfair a=losfair ref: #713 Co-authored-by: losfair <[email protected]>
This PR includes:
BlockTrace
middleware, and support for reading states of previous WebAssembly stack frames from middlewares.