-
Notifications
You must be signed in to change notification settings - Fork 303
add blog post "Stabilizing naked functions" #1650
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: master
Are you sure you want to change the base?
Conversation
d01096f
to
caea311
Compare
Hi, noticed this while reviewing the 1.88 relnotes (#1651 (comment)). I feel like this should be a public Blog Post, because this is about the stabilized user-facing language feature, especially about "well, why do we need this language feature in the first place"? Sure, it has some tidbits on the implementation details on how we got to stabilized naked functions, and some possible future directions, but I think that might also be interesting for the community at large :) |
caea311
to
1dbdd38
Compare
Sure, I've adjusted it, and made some further small tweaks/typo fixes. |
63ddac4
to
e323c5b
Compare
This looks good to me. 👍 🚢 |
e323c5b
to
6e8b140
Compare
Made some final tweaks
With that, I think this is ready, but it looks like it needs a re-approve? |
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.
A handful of optional comments from one final read through, but overall this looks pretty great!
#[unsafe(naked)] | ||
pub unsafe extern "custom" fn __aeabi_idivmod() { | ||
core::arch::naked_asm!( | ||
"push {{r0, r1, r4, lr}}", | ||
"bl {trampoline}", | ||
"pop {{r1, r2}}", | ||
"muls r2, r2, r0", | ||
"subs r1, r1, r2", | ||
"pop {{r4, pc}}", | ||
trampoline = sym crate::arm::__aeabi_idiv, | ||
); | ||
} |
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.
This suggestion might be a bit much since this section is basically an addendum, but it's probably worth adding at least a couple of hints
/// Division and modulo of two numbers using Arm's nonstandard ABI.
///
/// ```c
/// typedef struct { int quot; int rem; } idiv_return;
/// __value_in_regs idiv_return __aeabi_idivmod(int num, int denom);
/// ```
// SAFETY: the assembly implements the expected ABI, and "custom" ensures this
// cannot be called directly.
#[unsafe(naked)]
pub unsafe extern "custom" fn __aeabi_idivmod() {
core::arch::naked_asm!(
"push {{r0, r1, r4, lr}}", // Back up clobbers
"bl {trampoline}", // Call an `extern "C"` function for a / b
"pop {{r1, r2}}",
"muls r2, r2, r0", // Perform the modulo
"subs r1, r1, r2",
"pop {{r4, pc}}", // Restore clobbers, implicit return by setting `pc`
trampoline = sym crate::arm::__aeabi_idiv,
);
}
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.
If we're giving detail we may as well give all the detail. Also maybe backport this to compiler-builtins
itself?
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 had the same thought, I'll probably update that whole module next time I have a reason to touch it
6e8b140
to
16b4124
Compare
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 great to me as well!
Providing an introduction to naked functions, some background on the 10-year journey to stabilization, and some future features related to inline assembly.
cc @tgross35 @joshtriplett
Rendered