rustc_codegen_llvm: Feature Conversion Tidying

[a4lg]

The author thinks we can improve to_llvm_features, a function to convert a Rust target feature name into an LLVM feature (or nothing, to ignore features unsupported by LLVM) for better maintainability.

1. We can simplify some clauses and some expressions.
2. There are some readability issues.

This PR attempts to resolve some of them by tidying many cases.

---

OUTDATED CONTENTS FOLLOW

Commit Groups

There are three kinds of changes in this PR:

1. Simplify certain places (Commit 1–3)
   I think merging them should be (simply) okay.
2. Merge match clauses (Commit 4)
   While many would accept, some changes might break the intent of the original author(s).
3. Reorder conversion cases (Commit 5)
   I'll describe this later.

To reviewers: I would like to hear which commits you can accept (if not all).

Reordering (Commit 5)

While match clauses of the current implementation seem too random, I think there are multiple choices to take.

1. Just sort by the architecture.
   It's simpler and we don't need to separate avx10.1 and avx10.2 into two different subsections. However, we will have similar match clauses for pretty much the same purpose (to filter out features unsupported by LLVM) in multiple places.
2. First sort by the conversion category and then the architecture (as in this PR).
   If we go with it, there are some cases we will have to separate similar target features into multiple categories (like avx10.1 with simple renaming and avx10.2 with LLVM version filter + renaming (considered complex) in x86). Instead, we will have a clear view of what's going to happen on the specific target feature.

I chose (2) but (1) is definitely a good option, too.

[rustbot]

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[workingjubilee]

Why do you switch from "The author" 3rd-person-self-reference to "I"?

Is this anything but just churn?

Did you ask a machine to output the PR description text according to a prompt and then paste it in?

View changes since this review

[workingjubilee]

As someone who has made several PRs and landed them, most of them on an arch-by-arch basis, you should already know this organization strategy doesn't make sense.

[workingjubilee]

r? @workingjubilee

[a4lg]

Why do you switch from "The author" 3rd-person-self-reference to "I"?

Is this anything but just churn?

Did you ask a machine to output the PR description text according to a prompt and then paste it in?

View changes since this review

As a commit message, I would prefer 3rd person self "the author" and "I" are only used inside homu-ignore block (sorry if I am missing something).

[a4lg]

@workingjubilee What about commits 1 through 3? If they are working for you, I will separate this PR into (a) commit 1–3 and (b) heavily modified commit 5 (sorted only by the architecture).

[workingjubilee]

6361abc looks fine.

The merged matches are probably fine.

Otherwise my remark about comparing against the 3-tuple instead of .0 applies: the .0 doesn't make it clear that it's comparing against the major of a (major, minor, patch) tuple.

[a4lg]

As someone who has made several PRs and landed them, most of them on an arch-by-arch basis, you should already know this organization strategy doesn't make sense.

Understood, taking another approach (sort only by the architecture) in the next submission.

[workingjubilee]

If you personally prefer something like .0 then I would be fine with us replacing the tuple with a struct with proper names so it's just something like llvm_version().major, or a better-named function that only returns the major version of LLVM, or essentially any form you think looks good aside from the fairly nondescript .0

[a4lg]

6361abc looks fine.

The merged matches are probably fine.

Otherwise my remark about comparing against the 3-tuple instead of .0 applies: the .0 doesn't make it clear that it's comparing against the major of a (major, minor, patch) tuple.

Hmm, I currently prefer the commit 2 for consistency with other clauses (there's no LLVM 20.0.x and comparison with (20, 0, 0) is just a major version comparison) but I see the point here.
For that part, we could make another PR (making all .0 comparison more clearer) but for now (and in this PR), I want to keep the commits 2 and 3 as-is.

I changed the PR to use dedicated variable (major) storing the major version of LLVM to check.

[a4lg]

Update (reflected comments by @workingjubilee):

• Inserted new commit, extracting the major version of LLVM (as major).
• For quicker review, the original commit 1–3 are rebased on it (with/without slight modification).
• Original commit 4 is abandoned and removed.
• Original commit 5 is removed for now (will be proposed as a separate PR).

[a4lg]

I'll wait for status update regarding #145071, which will heavily affect this PR.

[bors]

☔ The latest upstream changes (presumably #146666) made this pull request unmergeable. Please resolve the merge conflicts.

[rustbot]

This PR was rebased onto a different master 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.

[a4lg]

Update: After #145071 is merged, this PR got a lot smaller. I added a commit to sort match clauses with the architecture (originally commit 5 but I took approach 1 (sort only by arch) instead of 2 (sort by category, then arch) in the initial submission).

[workingjubilee]

Thanks.

@bors r+ rollup

[bors]

📌 Commit a1a3cd0 has been approved by workingjubilee

It is now in the queue for this repository.