rustc_codegen_llvm: Feature Conversion Tidying by a4lg · Pull Request #146615 · rust-lang/rust (original) (raw)

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.