Skip to content

remove the simple_llvm_intrinsic wrappers #10730

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

Closed
wants to merge 1 commit into from
Closed

remove the simple_llvm_intrinsic wrappers #10730

wants to merge 1 commit into from

Conversation

thestinger
Copy link
Contributor

This removes a layer of generated wrappers from native LLVM intrinsics
we can represent directly in the type system. There are a lot with i1
parameters so there's still a need for those to be in the compiler.

These are an implementation detail of the compiler, so the inconsistency
is worth the simplification and faster compiles.

@alexcrichton
Copy link
Member

I've been on the fence about this for awhile. I think this is actually the third time I've seen this come up! I certainly like the negative diffstat on this, but I do think that there's a possibly major downside to this. As you've mentioned in #10008, none of these intrinsics are really unsafe at all (at least all the math ones). If we move to using link_name instead of compiler support, then there won't be any way to expose these as safe bindings to the equivalent LLVM functions.

It also seems a little odd to have half the intrinsics in the compiler and half of them using link_name, but that's not really a reason to not land this. What do you think about not being able to flag these as safe?

@thestinger
Copy link
Contributor Author

@alexcrichton: I think forcing all extern block functions to unsafe should be done with a default deny lint check so the minority of foreign functions that are safe for all inputs can still be exposed as safe. Of course, this isn't going to work for automatically generated C bindings via libclang, but it will work for these.

#10267

C and LLVM functions don't match the Rust ABI since they're missing an environment pointer so for now you still do need a wrapper anyway... but at least most of those wrappers are already there to deal with the current unsafe issue. If we drop the environment pointer they can be directly exposed.

@alexcrichton
Copy link
Member

Interesting, I feel like this pull request should have the same fate as #10267 in that case, I will nominate that for discussion.

This removes a layer of generated wrappers from native LLVM intrinsics
we can represent directly in the type system. There are a lot with `i1`
parameters so there's still a need for those to be in the compiler.

These are an implementation detail of the compiler, so the inconsistency
is worth the simplification and faster compiles.
@brson
Copy link
Contributor

brson commented Dec 4, 2013

This does prevent the standard library from being built with a hypothetical non-LLVM backend (or at least the intrinsics need to be declared differently per backend).

@thestinger
Copy link
Contributor Author

Yeah, it makes this implementation of the standard library less portable, but the core language definition becomes smaller so an alternate implementation would have a bit less to do.

I would like to feature-gate "rust-intrinsic" and #[link_name = "llvm.*"] and hide the numeric intrinsics as implementation details of the numeric modules. The interface could then be implemented as an LLVM intrinsic, libm call or gcc intrinsic without changing any user-visible APIs. I have an RFC open about part of this: #10008.

@emberian
Copy link
Member

A hearty 👍 on this

@alexcrichton
Copy link
Member

We have decided to close #10267, and as a result I agree with @brson that using the llvm.foo.bar names ties us a little too closely to LLVM if anyone ever wants to create another backend for rust. This isn't a huge burden in the compiler as intrinsics are well supported. As a result, I'm closing this.

@thestinger thestinger deleted the intrinsic branch January 15, 2014 08:44
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 5, 2023
`imprecise_flops`: Globally ignore `#[no_std]` crates

Really small fix.
Fixes rust-lang#10728
changelog: [`imprecise_flops`]: Fix false positives with `#[no_std]`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants