Skip to content
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

Cleanup: Use #[rust_intrinsic] and nix rust-intrinsic et. al #63585

Closed
Centril opened this issue Aug 15, 2019 · 7 comments
Closed

Cleanup: Use #[rust_intrinsic] and nix rust-intrinsic et. al #63585

Centril opened this issue Aug 15, 2019 · 7 comments
Labels
A-intrinsics Area: Intrinsics C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Aug 15, 2019

Per @eddyb's suggestion (rust-lang/reference#652 (comment)):

Not sure these should be documented, we've been meaning to remove all 3 for years...
cc @oli-obk at least for intrinsics we could maybe have an intermediary stage where we do #[rustc_intrinsic] extern "Rust" {...} instead of extern "rust-intrinsic" {...}?

I would maybe call them "pseudo-ABIs", in that they use the ABI string to indicate something special not purporting to the ABI, but are otherwise the "Rust" ABI.

cc @oli-obk, @nagisa, and @nikomatsakis

@Centril Centril added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-intrinsics Area: Intrinsics labels Aug 15, 2019
@eddyb
Copy link
Member

eddyb commented Aug 15, 2019

Ideally these would be regular declarations (empty or self-calling, if we want to get clever about it), marked with some attribute (like lang items), without extern { ... } blocks at all.

But we've been waiting on that for years, so maybe splitting it into steps will help.

@fstirlitz
Copy link

'Not purporting to the ABI' may be an overstatement. The mechanism by which a compiler realises function calls is usually called an ABI after all (or a 'calling convention'). For regular functions this means mangling the name and putting a call instruction into the caller's body; whereas for intrinsics it means looking up the intrinsic in a table built into the compiler and putting some operations associated with it into the caller's body (which may not be otherwise expressible in surface syntax). I don't see what the problem is in calling both mechanisms 'ABIs'.

(And yes, that means inlining is an alternative calling convention as well. The way inlining is usually implemented in programming languages is just a matter of convenience; if instead of an attribute like #[inline], we had extern "inline" and no ability to form pointers to such functions, they would be much less expedient. But I doubt this is ever an issue with intrinsics.)

@eddyb
Copy link
Member

eddyb commented Aug 15, 2019

@fstirlitz In all other situations, the "ABI string" (after the extern keyword) controls the transfer of values to and from (i.e. arguments and return value) the callee, but not how the function is found.

You mention mangling: mangling is not affected in any way by that "ABI string".
(extern "ABI" shouldn't have been the syntax, because AFAIK all it controls in C++ is mangling...)

@nagisa
Copy link
Member

nagisa commented Aug 15, 2019

Agreed that we want intrinsics to be plain function definitions with an attribute.

#[rustc_intrinsic]
fn foobar(...) -> ... { 
    /* whatever works, body does not matter, empty `loop {}` or self-call all work */
}

It is perhaps a nice thing to make function pointers to intrinsics work; having intrinsics to be function definitions that self-call would make it "just work", solving a long-standing ICE with intrinsics.

@Centril
Copy link
Contributor Author

Centril commented Aug 15, 2019

Would be good to note what gains we expect from this other than making function pointers to intrinsics work (why is that important anyways?...).

@eddyb
Copy link
Member

eddyb commented Aug 16, 2019

@Centril using FFI imports causes all sorts of undesireable behavior, such as all intrinsics being unsafe and their const fn-ness being a complete hack.

There are situations where we have a safe wrapper (or several, in the case of generic intrinsics, especially those generic over integer/float primitives) around an unsafe intrinsic, and we would prefer to mark that wrapper as the intrinsic.

Bonus: we can get rid of some post-monomorphization errors/ICEs if all of the intrinsics generic over a finite set of types (e.g. wrapping_add only works with iN and uN types) are instead defined non-generically and thus can be type-checked early.

oli-obk added a commit to oli-obk/rust that referenced this issue Feb 13, 2024
Implement intrinsics with fallback bodies

fixes rust-lang#93145 (though we can port many more intrinsics)
cc rust-lang#63585

The way this works is that the backend logic for generating custom code for intrinsics has been made fallible. The only failure path is "this intrinsic is unknown". The `Instance` (that was `InstanceDef::Intrinsic`) then gets converted to `InstanceDef::Item`, which represents the fallback body. A regular function call to that body is then codegenned. This is currently implemented for

* codegen_ssa (so llvm and gcc)
* codegen_cranelift

other backends will need to adjust, but they can just keep doing what they were doing if they prefer (though adding new intrinsics to the compiler will then require them to implement them, instead of getting the fallback body).

cc `@scottmcm` `@WaffleLapkin`

### todo

* [ ] miri support
* [x] default intrinsic name to name of function instead of requiring it to be specified in attribute
* [x] make sure that the bodies are always available (must be collected for metadata)
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 13, 2024
Implement intrinsics with fallback bodies

fixes rust-lang#93145 (though we can port many more intrinsics)
cc rust-lang#63585

The way this works is that the backend logic for generating custom code for intrinsics has been made fallible. The only failure path is "this intrinsic is unknown". The `Instance` (that was `InstanceDef::Intrinsic`) then gets converted to `InstanceDef::Item`, which represents the fallback body. A regular function call to that body is then codegenned. This is currently implemented for

* codegen_ssa (so llvm and gcc)
* codegen_cranelift

other backends will need to adjust, but they can just keep doing what they were doing if they prefer (though adding new intrinsics to the compiler will then require them to implement them, instead of getting the fallback body).

cc ``@scottmcm`` ``@WaffleLapkin``

### todo

* [ ] miri support
* [x] default intrinsic name to name of function instead of requiring it to be specified in attribute
* [x] make sure that the bodies are always available (must be collected for metadata)
geektype added a commit to geektype/rust that referenced this issue Feb 13, 2024
Implement intrinsics with fallback bodies

fixes rust-lang#93145 (though we can port many more intrinsics)
cc rust-lang#63585

The way this works is that the backend logic for generating custom code for intrinsics has been made fallible. The only failure path is "this intrinsic is unknown". The `Instance` (that was `InstanceDef::Intrinsic`) then gets converted to `InstanceDef::Item`, which represents the fallback body. A regular function call to that body is then codegenned. This is currently implemented for

* codegen_ssa (so llvm and gcc)
* codegen_cranelift

other backends will need to adjust, but they can just keep doing what they were doing if they prefer (though adding new intrinsics to the compiler will then require them to implement them, instead of getting the fallback body).

cc ```@scottmcm``` ```@WaffleLapkin```

### todo

* [ ] miri support
* [x] default intrinsic name to name of function instead of requiring it to be specified in attribute
* [x] make sure that the bodies are always available (must be collected for metadata)
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 15, 2024
Implement intrinsics with fallback bodies

fixes rust-lang#93145 (though we can port many more intrinsics)
cc rust-lang#63585

The way this works is that the backend logic for generating custom code for intrinsics has been made fallible. The only failure path is "this intrinsic is unknown". The `Instance` (that was `InstanceDef::Intrinsic`) then gets converted to `InstanceDef::Item`, which represents the fallback body. A regular function call to that body is then codegenned. This is currently implemented for

* codegen_ssa (so llvm and gcc)
* codegen_cranelift

other backends will need to adjust, but they can just keep doing what they were doing if they prefer (though adding new intrinsics to the compiler will then require them to implement them, instead of getting the fallback body).

cc `@scottmcm` `@WaffleLapkin`

### todo

* [ ] miri support
* [x] default intrinsic name to name of function instead of requiring it to be specified in attribute
* [x] make sure that the bodies are always available (must be collected for metadata)
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2024
Implement intrinsics with fallback bodies

fixes rust-lang#93145 (though we can port many more intrinsics)
cc rust-lang#63585

The way this works is that the backend logic for generating custom code for intrinsics has been made fallible. The only failure path is "this intrinsic is unknown". The `Instance` (that was `InstanceDef::Intrinsic`) then gets converted to `InstanceDef::Item`, which represents the fallback body. A regular function call to that body is then codegenned. This is currently implemented for

* codegen_ssa (so llvm and gcc)
* codegen_cranelift

other backends will need to adjust, but they can just keep doing what they were doing if they prefer (though adding new intrinsics to the compiler will then require them to implement them, instead of getting the fallback body).

cc `@scottmcm` `@WaffleLapkin`

### todo

* [ ] miri support
* [x] default intrinsic name to name of function instead of requiring it to be specified in attribute
* [x] make sure that the bodies are always available (must be collected for metadata)
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 22, 2024
Add a scheme for moving away from `extern "rust-intrinsic"` entirely

All `rust-intrinsic`s can become free functions now, either with a fallback body, or with a dummy body and an attribute, requiring backends to actually implement the intrinsic.

This PR demonstrates the dummy-body scheme with the `vtable_size` intrinsic.

cc rust-lang#63585

follow-up to rust-lang#120500

MCP at rust-lang/compiler-team#720
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 22, 2024
Add a scheme for moving away from `extern "rust-intrinsic"` entirely

All `rust-intrinsic`s can become free functions now, either with a fallback body, or with a dummy body and an attribute, requiring backends to actually implement the intrinsic.

This PR demonstrates the dummy-body scheme with the `vtable_size` intrinsic.

cc rust-lang#63585

follow-up to rust-lang#120500

MCP at rust-lang/compiler-team#720
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 23, 2024
Add a scheme for moving away from `extern "rust-intrinsic"` entirely

All `rust-intrinsic`s can become free functions now, either with a fallback body, or with a dummy body and an attribute, requiring backends to actually implement the intrinsic.

This PR demonstrates the dummy-body scheme with the `vtable_size` intrinsic.

cc rust-lang#63585

follow-up to rust-lang#120500

MCP at rust-lang/compiler-team#720
flip1995 pushed a commit to flip1995/rust that referenced this issue Feb 26, 2024
Implement intrinsics with fallback bodies

fixes rust-lang#93145 (though we can port many more intrinsics)
cc rust-lang#63585

The way this works is that the backend logic for generating custom code for intrinsics has been made fallible. The only failure path is "this intrinsic is unknown". The `Instance` (that was `InstanceDef::Intrinsic`) then gets converted to `InstanceDef::Item`, which represents the fallback body. A regular function call to that body is then codegenned. This is currently implemented for

* codegen_ssa (so llvm and gcc)
* codegen_cranelift

other backends will need to adjust, but they can just keep doing what they were doing if they prefer (though adding new intrinsics to the compiler will then require them to implement them, instead of getting the fallback body).

cc `@scottmcm` `@WaffleLapkin`

### todo

* [ ] miri support
* [x] default intrinsic name to name of function instead of requiring it to be specified in attribute
* [x] make sure that the bodies are always available (must be collected for metadata)
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 4, 2024
Add a scheme for moving away from `extern "rust-intrinsic"` entirely

All `rust-intrinsic`s can become free functions now, either with a fallback body, or with a dummy body and an attribute, requiring backends to actually implement the intrinsic.

This PR demonstrates the dummy-body scheme with the `vtable_size` intrinsic.

cc rust-lang#63585

follow-up to rust-lang#120500

MCP at rust-lang/compiler-team#720
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 5, 2024
Add a scheme for moving away from `extern "rust-intrinsic"` entirely

All `rust-intrinsic`s can become free functions now, either with a fallback body, or with a dummy body and an attribute, requiring backends to actually implement the intrinsic.

This PR demonstrates the dummy-body scheme with the `vtable_size` intrinsic.

cc rust-lang#63585

follow-up to rust-lang#120500

MCP at rust-lang/compiler-team#720
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Mar 7, 2024
Add a scheme for moving away from `extern "rust-intrinsic"` entirely

All `rust-intrinsic`s can become free functions now, either with a fallback body, or with a dummy body and an attribute, requiring backends to actually implement the intrinsic.

This PR demonstrates the dummy-body scheme with the `vtable_size` intrinsic.

cc rust-lang/rust#63585

follow-up to #120500

MCP at rust-lang/compiler-team#720
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 19, 2024
@RalfJung
Copy link
Member

RalfJung commented Nov 7, 2024

Status update: #120500 implemented the #[rustc_intrinsic] attribute, #132735 tracks using it everywhere.

I don't think this issue still has any reason to remain open, so I will close it.

@RalfJung RalfJung closed this as completed Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intrinsics Area: Intrinsics C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants