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

[DO NOT SUBMIT] rustc_codegen_llvm: adapt for LLVM 20 changes #129894

Closed
wants to merge 2 commits into from

Conversation

krasimirgg
Copy link
Contributor

@krasimirgg krasimirgg commented Sep 2, 2024

No functional changes intended.

LLVM 20 uses a different name for this feature.

@rustbot label: +llvm-main
r? @nikic

@rustbot
Copy link
Collaborator

rustbot commented Sep 2, 2024

Failed to set assignee to mrkajetanp: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) labels Sep 2, 2024
@krasimirgg
Copy link
Contributor Author

cc: @mrkajetanp

@rust-log-analyzer

This comment has been minimized.

No functional changes intended.

LLVM 20 uses a different name for this feature.
@mrkajetanp
Copy link
Contributor

mrkajetanp commented Sep 2, 2024

To update this for LLVM 20 there will need to be a change to std_detect in stdarch first, here:
https://github.com/rust-lang/stdarch/blob/master/crates/std_detect/src/detect/os/linux/aarch64.rs#L478
As currently implemented, sve-b16b16 in rustc is a "joint feature" detected by the presence of either the SVE field or the SME field.
The LLVM 20 update is splitting bf1616 into two, not just renaming it. sme-b16b16 needs to be added in stdarch as a proper feature, the detection logic updated to detect each of them separately and only then to_llvm_features should be updated on this side. As I mentioned I'm planning on doing that this week anyway, just wanna make sure the LLVM split doesn't mess anything up first.

See this LLVM change:
llvm/llvm-project@1b936e4

@krasimirgg krasimirgg changed the title rustc_codegen_llvm: adapt for LLVM 20 changes [DO NOT SUBMIT] rustc_codegen_llvm: adapt for LLVM 20 changes Sep 2, 2024
@krasimirgg
Copy link
Contributor Author

r? @ghost

@rustbot
Copy link
Collaborator

rustbot commented Sep 2, 2024

Failed to set assignee to ghost: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Oct 10, 2024
rustc_target: Add sme-b16b16 as an explicit aarch64 target feature

LLVM 20 split out what used to be called b16b16 and correspond to aarch64
FEAT_SVE_B16B16 into sve-b16b16 and sme-b16b16.
Add sme-b16b16 as an explicit feature and update the codegen accordingly.

Resolves rust-lang#129894.
@bors bors closed this in edb6693 Oct 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2024
Rollup merge of rust-lang#130741 - mrkajetanp:detect-b16b16, r=Amanieu

rustc_target: Add sme-b16b16 as an explicit aarch64 target feature

LLVM 20 split out what used to be called b16b16 and correspond to aarch64
FEAT_SVE_B16B16 into sve-b16b16 and sme-b16b16.
Add sme-b16b16 as an explicit feature and update the codegen accordingly.

Resolves rust-lang#129894.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants