Conversation
rustbot
added
A-compiler-builtins
|
Error: Failed to set assignee to
Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
| intrinsics! { | ||
| #[maybe_use_optimized_c_shim] | ||
| #[unsafe(naked)] | ||
| #[target_feature(enable = "lse")] |
There was a problem hiding this comment.
Just for the record, as of recently, on LLVM 23, naked functions do actually support target features
But it looks like there are other reasons to prefer asm! here.
| intrinsics! { | ||
| #[maybe_use_optimized_c_shim] | ||
| #[unsafe(naked)] | ||
| #[target_feature(enable = "lse")] |
There was a problem hiding this comment.
Doesn't this enable lse before we know it's enabled? Since HAVE_LSE_ATOMICS hasn't been checked
There was a problem hiding this comment.
I think this should use .arch_extension lse in the relevant block instead
There was a problem hiding this comment.
I am ok with making the change. Though, how would target_feature's behavior differ?
There was a problem hiding this comment.
Oh, is that potentially gated by the active target configuration.
There was a problem hiding this comment.
I am ok with making the change. Though, how would target_feature's behavior differ?
Not sure I'm understanding the question right but I believe that #[target_feature(enable = "lse")] can be thought of as saying "LSE is known to be enabled for this function, and it is unsound to call if that isn't the case". Since it means LLVM is allowed to use LSE for the whole function, whereas .arch_extension lse just keeps the assembler from rejecting LSE instructions in that single block without further requirements.
Probably not going to make a difference in codegen here but it's more accurate since HAVE_LSE_ATOMICS could be false.
|
Also when this is closer to ready it should ideally be filed against https://github.com/rust-lang/compiler-builtins, that testsuite still isn't run as part of r-l/r |
The outlined atomics should follow the AAPCS64 calling conventions, and allow usage of -Z branch-protection=bti. We just need to be careful to avoid bad codegen. This generates almost identical codegen with -C opt-level=1. Those differences are entirely regalloc choices.
The in/inlateout register operands aren't entirely accurate (w vs x). This annoys me, but it's the same register, and we cannot use a macro inside that portion of asm!.
And, remove unused macros.
ddce59f to
982f137
Compare
|
Moving this over to rust-lang/compiler-builtins#1321. |
rustbot
removed
the
S-waiting-on-author
4 participants
I think this is what was suggested in rust-lang/compiler-builtins#1063, and maybe step towards resolving #151486.
The cas16 operations are the most complicated. This compiles into nearly identical machine code.
r? @taiki-e @tgross35