Codegen Overloaded LLVM intrinsics based on their name - #157145
Conversation
This comment has been minimized.
This comment has been minimized.
a399a07 to
c55c137
Compare
This comment has been minimized.
This comment has been minimized.
c55c137 to
36e3bbc
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
cc @rust-lang/wg-llvm could you have a look here - when you have some time? thanks |
I will review this PR this week. |
|
@bors r+ |
…ianqk Codegen Overloaded LLVM intrinsics based on their name This is a continuation of rust-lang#140763 - now codegenning overloaded LLVM intrinsics based on their name too. This PR parses the `link_name` of the LLVM intrinsics for the type parameters, partially inverting [`getMangledTypeStr`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00076) and [`getIntrinsicNameImpl`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00165) from LLVM. There is the concern that @nikic's work on LLVM intrinsics might remove the name mangling, but we can just retain that from the Rust side. I mean even though the LLVM IR wouldn't have the mangling, but we can require that the Rust `link_name` argument contain the mangling. This shouldn't break anything, as existing code already has the name mangling. There is also the concern that this cannot parse `TargetExt` types and non-literal struct types, as their mangling contains their name. If needed in future, we can maybe hardcode some known `TargetExt` types, but currently we don't support it. It also kinda helps that Rust currently cannot handle `TargetExt` types. The named struct one is not that big of a problem because courtesy of rust-lang#140763 we can already repack structs. I have not added support of LLVM `byte` type because it is only available in LLVM22, and we support min-LLVM version 20 afaik. I prefer this approach over the `IITDesc` approach highlighted in rust-lang#140763 because this approach allows code like ```rust #[link_name = "llvm.sqrt.v8bf16"] fn foo(a: u16x8) -> u16x8; ``` which pairs up with the autocasts of rust-lang#140763 to give a nice way to call overloaded intrinsics on `bf16`. Also this approach is a lot less work and more resilient to LLVM changes than the `IITDesc` approach. One important change - the parsing doesn't account for LLVM typed pointers, which were deprecated in LLVM15 and removed in LLVM17, so I didn't bother putting support for them. So, I also removed all uses of typed pointers from the tree. r? @dianqk as you might have more of a context on this due to reviewing the last 2 PRs cc @nikic
…ianqk Codegen Overloaded LLVM intrinsics based on their name This is a continuation of rust-lang#140763 - now codegenning overloaded LLVM intrinsics based on their name too. This PR parses the `link_name` of the LLVM intrinsics for the type parameters, partially inverting [`getMangledTypeStr`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00076) and [`getIntrinsicNameImpl`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00165) from LLVM. There is the concern that @nikic's work on LLVM intrinsics might remove the name mangling, but we can just retain that from the Rust side. I mean even though the LLVM IR wouldn't have the mangling, but we can require that the Rust `link_name` argument contain the mangling. This shouldn't break anything, as existing code already has the name mangling. There is also the concern that this cannot parse `TargetExt` types and non-literal struct types, as their mangling contains their name. If needed in future, we can maybe hardcode some known `TargetExt` types, but currently we don't support it. It also kinda helps that Rust currently cannot handle `TargetExt` types. The named struct one is not that big of a problem because courtesy of rust-lang#140763 we can already repack structs. I have not added support of LLVM `byte` type because it is only available in LLVM22, and we support min-LLVM version 20 afaik. I prefer this approach over the `IITDesc` approach highlighted in rust-lang#140763 because this approach allows code like ```rust #[link_name = "llvm.sqrt.v8bf16"] fn foo(a: u16x8) -> u16x8; ``` which pairs up with the autocasts of rust-lang#140763 to give a nice way to call overloaded intrinsics on `bf16`. Also this approach is a lot less work and more resilient to LLVM changes than the `IITDesc` approach. One important change - the parsing doesn't account for LLVM typed pointers, which were deprecated in LLVM15 and removed in LLVM17, so I didn't bother putting support for them. So, I also removed all uses of typed pointers from the tree. r? @dianqk as you might have more of a context on this due to reviewing the last 2 PRs cc @nikic
…ianqk Codegen Overloaded LLVM intrinsics based on their name This is a continuation of rust-lang#140763 - now codegenning overloaded LLVM intrinsics based on their name too. This PR parses the `link_name` of the LLVM intrinsics for the type parameters, partially inverting [`getMangledTypeStr`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00076) and [`getIntrinsicNameImpl`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00165) from LLVM. There is the concern that @nikic's work on LLVM intrinsics might remove the name mangling, but we can just retain that from the Rust side. I mean even though the LLVM IR wouldn't have the mangling, but we can require that the Rust `link_name` argument contain the mangling. This shouldn't break anything, as existing code already has the name mangling. There is also the concern that this cannot parse `TargetExt` types and non-literal struct types, as their mangling contains their name. If needed in future, we can maybe hardcode some known `TargetExt` types, but currently we don't support it. It also kinda helps that Rust currently cannot handle `TargetExt` types. The named struct one is not that big of a problem because courtesy of rust-lang#140763 we can already repack structs. I have not added support of LLVM `byte` type because it is only available in LLVM22, and we support min-LLVM version 20 afaik. I prefer this approach over the `IITDesc` approach highlighted in rust-lang#140763 because this approach allows code like ```rust #[link_name = "llvm.sqrt.v8bf16"] fn foo(a: u16x8) -> u16x8; ``` which pairs up with the autocasts of rust-lang#140763 to give a nice way to call overloaded intrinsics on `bf16`. Also this approach is a lot less work and more resilient to LLVM changes than the `IITDesc` approach. One important change - the parsing doesn't account for LLVM typed pointers, which were deprecated in LLVM15 and removed in LLVM17, so I didn't bother putting support for them. So, I also removed all uses of typed pointers from the tree. r? @dianqk as you might have more of a context on this due to reviewing the last 2 PRs cc @nikic
…ianqk Codegen Overloaded LLVM intrinsics based on their name This is a continuation of rust-lang#140763 - now codegenning overloaded LLVM intrinsics based on their name too. This PR parses the `link_name` of the LLVM intrinsics for the type parameters, partially inverting [`getMangledTypeStr`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00076) and [`getIntrinsicNameImpl`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00165) from LLVM. There is the concern that @nikic's work on LLVM intrinsics might remove the name mangling, but we can just retain that from the Rust side. I mean even though the LLVM IR wouldn't have the mangling, but we can require that the Rust `link_name` argument contain the mangling. This shouldn't break anything, as existing code already has the name mangling. There is also the concern that this cannot parse `TargetExt` types and non-literal struct types, as their mangling contains their name. If needed in future, we can maybe hardcode some known `TargetExt` types, but currently we don't support it. It also kinda helps that Rust currently cannot handle `TargetExt` types. The named struct one is not that big of a problem because courtesy of rust-lang#140763 we can already repack structs. I have not added support of LLVM `byte` type because it is only available in LLVM22, and we support min-LLVM version 20 afaik. I prefer this approach over the `IITDesc` approach highlighted in rust-lang#140763 because this approach allows code like ```rust #[link_name = "llvm.sqrt.v8bf16"] fn foo(a: u16x8) -> u16x8; ``` which pairs up with the autocasts of rust-lang#140763 to give a nice way to call overloaded intrinsics on `bf16`. Also this approach is a lot less work and more resilient to LLVM changes than the `IITDesc` approach. One important change - the parsing doesn't account for LLVM typed pointers, which were deprecated in LLVM15 and removed in LLVM17, so I didn't bother putting support for them. So, I also removed all uses of typed pointers from the tree. r? @dianqk as you might have more of a context on this due to reviewing the last 2 PRs cc @nikic
| pub unsafe fn overloaded_bf16_autocast(a: i16x8) -> i16x8 { | ||
| extern "unadjusted" { | ||
| #[link_name = "llvm.sqrt.v8bf16"] | ||
| fn foo(a: i16x8) -> i16x8; |
There was a problem hiding this comment.
I find this extremely confusing. What is going on here? The intrinsic actually has type <8 x bfloat> but we pass it an i16x8? Why should we allow such code...?
There was a problem hiding this comment.
I think the main motivation for this entire approach was ability to insert magic type casts for types that Rust does not support.
Of course, this is kind of moot with f16b being introduced in #160859.
(FWIW, I don't think what this PR does is a good idea, but I don't have time to fight it.)
There was a problem hiding this comment.
Of course, this is kind of moot with f16b being introduced in #160859.
Yeah that seems to remove this part of the motivation. OTOH the old referenced PR also mentions
LLVM intrinsics that have types in their signature that can't be accessed from Rust (notable examples are the AMX intrinsics that have the x86amx type, and (almost) all intrinsics that have vectors of i1 types) can't be linked to at all. This is a (major?) roadblock in the AMX and AVX512 support in stdarch.
So the question is, will we have native types for those as well? We might have to, if they are relevant for ABIs we need to implement. But if they are only used for LLVM intrinsics, not for extern "C", then this PR may save us from adding that type. I don't know anything about x86amx, or whether there's a better way to deal with vectors of i1.
Another motivation seems to be dealing with LLVM intrinsics that change their signature -- how is stdarch supposed to import these? But this does works today so it seems the LLVM auto-upgrade for this is enough?
And then there's "finding signatures that are just wrong". But it seems that already landed in #140763.
On the list of downsides we have "keeping the long mangled LLVM names even after LLVM itself doesn't use them any more", if I understood correctly the plan for how to deal with @nikic's work on getting rid of the name mangling in LLVM. That seems like a silly end state and is clearly path-dependent -- if LLVM had removed the mangling 5 years ago, I doubt we'd accept a PR like this to add magic name mangling in the link_name string to indicate the "real" signature. So likewise I don't think we want to be in a state where we are keeping the name mangling alive long after LLVM dropped it.
I also couldn't find an MCP for this change. Should it have one? It's very backend-internal, but OTOH one of our main backend experts disagrees so it may be good to have a bit of a wider discussion. @dianqk seems fine with it.
Rollup of 7 pull requests Successful merges: - #150075 (Implement clamp_to) - #157145 (Codegen Overloaded LLVM intrinsics based on their name) - #161866 (delegation: add tests fixating behavior of delegating to default trait implementations) - #161456 (reduce perf impact of scalar size checks) - #161730 (Improve type mismatch annotation for lets with block-wrapped initializers) - #161828 (Never type after-stabilization cleanup) - #161860 (atomicptr.rs test: remove unused import)
…ianqk Codegen Overloaded LLVM intrinsics based on their name This is a continuation of rust-lang#140763 - now codegenning overloaded LLVM intrinsics based on their name too. This PR parses the `link_name` of the LLVM intrinsics for the type parameters, partially inverting [`getMangledTypeStr`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00076) and [`getIntrinsicNameImpl`](https://llvm.org/doxygen/Intrinsics_8cpp_source.html#l00165) from LLVM. There is the concern that @nikic's work on LLVM intrinsics might remove the name mangling, but we can just retain that from the Rust side. I mean even though the LLVM IR wouldn't have the mangling, but we can require that the Rust `link_name` argument contain the mangling. This shouldn't break anything, as existing code already has the name mangling. There is also the concern that this cannot parse `TargetExt` types and non-literal struct types, as their mangling contains their name. If needed in future, we can maybe hardcode some known `TargetExt` types, but currently we don't support it. It also kinda helps that Rust currently cannot handle `TargetExt` types. The named struct one is not that big of a problem because courtesy of rust-lang#140763 we can already repack structs. I have not added support of LLVM `byte` type because it is only available in LLVM22, and we support min-LLVM version 20 afaik. I prefer this approach over the `IITDesc` approach highlighted in rust-lang#140763 because this approach allows code like ```rust #[link_name = "llvm.sqrt.v8bf16"] fn foo(a: u16x8) -> u16x8; ``` which pairs up with the autocasts of rust-lang#140763 to give a nice way to call overloaded intrinsics on `bf16`. Also this approach is a lot less work and more resilient to LLVM changes than the `IITDesc` approach. One important change - the parsing doesn't account for LLVM typed pointers, which were deprecated in LLVM15 and removed in LLVM17, so I didn't bother putting support for them. So, I also removed all uses of typed pointers from the tree. r? @dianqk as you might have more of a context on this due to reviewing the last 2 PRs cc @nikic
|
@bors try jobs=aarch64-gnu |
This comment has been minimized.
This comment has been minimized.
Codegen Overloaded LLVM intrinsics based on their name try-job: aarch64-gnu
|
@bors r- |
|
This pull request was unapproved. This PR was contained in a rollup (#161874), which was unapproved. |
|
💔 Test for 052f00a failed: CI. Failed job:
|
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #161398) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
This is a continuation of #140763 - now codegenning overloaded LLVM intrinsics based on their name too. This PR parses the
link_nameof the LLVM intrinsics for the type parameters, partially invertinggetMangledTypeStrandgetIntrinsicNameImplfrom LLVM.There is the concern that @nikic's work on LLVM intrinsics might remove the name mangling, but we can just retain that from the Rust side. I mean even though the LLVM IR wouldn't have the mangling, but we can require that the Rust
link_nameargument contain the mangling. This shouldn't break anything, as existing code already has the name mangling.There is also the concern that this cannot parse
TargetExttypes and non-literal struct types, as their mangling contains their name. If needed in future, we can maybe hardcode some knownTargetExttypes, but currently we don't support it. It also kinda helps that Rust currently cannot handleTargetExttypes. The named struct one is not that big of a problem because courtesy of #140763 we can already repack structs.I have not added support of LLVM
bytetype because it is only available in LLVM22, and we support min-LLVM version 20 afaik.I prefer this approach over the
IITDescapproach highlighted in #140763 because this approach allows code likewhich pairs up with the autocasts of #140763 to give a nice way to call overloaded intrinsics on
bf16. Also this approach is a lot less work and more resilient to LLVM changes than theIITDescapproach.One important change - the parsing doesn't account for LLVM typed pointers, which were deprecated in LLVM15 and removed in LLVM17, so I didn't bother putting support for them. So, I also removed all uses of typed pointers from the tree.
r? @dianqk as you might have more of a context on this due to reviewing the last 2 PRs
cc @nikic