-
Notifications
You must be signed in to change notification settings - Fork 16
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 support for environments with the FPU disabled #25
Conversation
src/lib.rs
Outdated
@@ -2,6 +2,7 @@ | |||
#![feature(c_unwind)] | |||
#![feature(naked_functions)] | |||
#![feature(non_exhaustive_omitted_patterns_lint)] | |||
#![feature(cfg_target_abi)] |
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.
Is there a way to achieve this without additional nightly features? I'd like to avoid adding extra ones.
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 it might be doable with target_feature
, but I'm not sure. Will do some testing and find out.
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.
Never mind, the counterparts in target_feature
are still unstable. The other option would be a cargo feature, but that feels weird. Your choice I guess.
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.
cfg_target_abi has been stabilized in the recent nightly (rust-lang/rust#119590), so we can now remove this #![feature]
src/unwinder/arch/aarch64.rs
Outdated
Register(64..=95) => &mut self.fp[(reg.0 - 64) as usize], | ||
_ => unimplemented!(), | ||
} | ||
} | ||
} | ||
|
||
#[naked] | ||
#[cfg(target_abi = "softfloat")] | ||
pub extern "C-unwind" fn save_context(f: extern "C" fn(&mut Context, *mut ()), ptr: *mut ()) { | ||
// No need to save caller-saved registers here. |
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.
Could you extract the asm using macro to avoid duplication? See riscv for example
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.
Yeah sure, will do.
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.
Please take another look of riscv code to avoid code duplication
As I said in my comments, I fixed these as much as I could. I need your input on the first one. |
Not sure how I closed this. Anyways, it's finally done. Sorry for the long wait. Hopefully this is what you envisioned. |
src/unwinder/arch/aarch64.rs
Outdated
unsafe { | ||
#[cfg(target_feature = "fp-armv8")] |
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.
AFAIK there is no such a target feature in aarch64 (at least on the Rust side):
$ rustc +nightly --print cfg --target aarch64-unknown-linux-gnu | grep target_feature
target_feature="neon"
$ rustc +nightly --print cfg --target aarch64-unknown-none | grep target_feature
target_feature="neon"
$ rustc +nightly --print cfg --target aarch64-unknown-none-softfloat | grep target_feature
arm has it though: https://github.com/rust-lang/rust/blob/b5b13568fb5da4ac988bde370008d6134d3dfe6c/compiler/rustc_target/src/target_features.rs#L62
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.
Oh, weird—the error when I tried to use unwinding
mentioned fp-armv8
explicitly. I'll fix that.
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.
To be more clear: when I tried to use the current unwinding
master branch on aarch64-unknown-none-softfloat
, I got a compile error saying that the registers d[n]
require target feature fp-armv8
. This is likely a diagnostic issue, and I'll see if I can reproduce it to send that over to rustc.
`fp-armv8` is an arm feature, not an aarch64 feature.
Currently, the crate causes an exception on systems where the FPU is off/nonexistent, because of this code segment that saves floating-point registers. This PR removes that floating-point saving on
-softfloat
targets.