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

target: default to the medium code model on LoongArch targets #120661

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

xen0n
Copy link
Contributor

@xen0n xen0n commented Feb 5, 2024

The Rust LoongArch targets have been using the default LLVM code model so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak. As described in the "Code Model" section of LoongArch ELF psABI spec v20231219, one can only make function calls as far as ±128MiB with the "normal" code model; this is insufficient for very large software containing Rust components that needs to be linked into the big text section, such as Chromium.

Because:

  • we do not want to ask users to recompile std if they are to build such software,
  • objects compiled with larger code models can be linked with those with smaller code models without problems, and
  • the "medium" code model is comparable to the "small"/"normal" one performance-wise (same data access pattern; each function call becomes 2-insn long and indirect, but this may be relaxed back into the direct 1-insn form in a future LLVM version), but is able to perform function calls within ±128GiB,

it is better to just switch the targets to the "medium" code model, which is also "medium" in LLVM-speak.

The Rust LoongArch targets have been using the default LLVM code model
so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak.
As described in the "Code Model" section of LoongArch ELF psABI spec
v20231219 [1], one can only make function calls as far as ±128MiB with
the "normal" code model; this is insufficient for very large software
containing Rust components that needs to be linked into the big text
section, such as Chromium.

Because:

* we do not want to ask users to recompile std if they are to build
  such software,
* objects compiled with larger code models can be linked with those
  with smaller code models without problems, and
* the "medium" code model is comparable to the "small"/"normal" one
  performance-wise (same data access pattern; each function call
  becomes 2-insn long and indirect, but this may be relaxed back into
  the direct 1-insn form in a future LLVM version), but is able to
  perform function calls within ±128GiB,

it is better to just switch the targets to the "medium" code model,
which is also "medium" in LLVM-speak.

[1]: https://github.com/loongson/la-abi-specs/blob/v2.30/laelf.adoc#code-models
@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 2024

r? @michaelwoerister

(rustbot has picked a reviewer for you, use r? to override)

@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 2024

These commits modify compiler targets.
(See the Target Tier Policy.)

@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. labels Feb 5, 2024
@xen0n
Copy link
Contributor Author

xen0n commented Feb 5, 2024

cc @heiher @xry111

also cc @jiegec -- after this patch is released and the new Rust version picked up by Chromium, the downstream Rust code-model change can be dropped from the patchset.

Copy link
Contributor

@heiher heiher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@Noratrieb
Copy link
Member

@bors r=heiher,Nilstrieb

@bors
Copy link
Contributor

bors commented Feb 5, 2024

📌 Commit 35dad14 has been approved by heiher,Nilstrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 5, 2024
…r,Nilstrieb

target: default to the medium code model on LoongArch targets

The Rust LoongArch targets have been using the default LLVM code model so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak. As [described][1] in the "Code Model" section of LoongArch ELF psABI spec v20231219, one can only make function calls as far as ±128MiB with the "normal" code model; this is insufficient for very large software containing Rust components that needs to be linked into the big text section, such as Chromium.

Because:

* we do not want to ask users to recompile std if they are to build such software,
* objects compiled with larger code models can be linked with those with smaller code models without problems, and
* the "medium" code model is comparable to the "small"/"normal" one performance-wise (same data access pattern; each function call becomes 2-insn long and indirect, but this may be relaxed back into the direct 1-insn form in a future LLVM version), but is able to perform function calls within ±128GiB,

it is better to just switch the targets to the "medium" code model, which is also "medium" in LLVM-speak.

[1]: https://github.com/loongson/la-abi-specs/blob/v2.30/laelf.adoc#code-models
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#113833 (`std::error::Error` -> Trait Implementations: lifetimes consistency improvement)
 - rust-lang#115386 (PartialEq, PartialOrd: update and synchronize handling of transitive chains)
 - rust-lang#116284 (make matching on NaN a hard error, and remove the rest of illegal_floating_point_literal_pattern)
 - rust-lang#118960 (Add LocalWaker and ContextBuilder types to core, and LocalWake trait to alloc.)
 - rust-lang#120384 (Use `<T, U>` for array/slice equality `impl`s)
 - rust-lang#120518 (riscv only supports split_debuginfo=off for now)
 - rust-lang#120619 (Assert that params with the same *index* have the same *name*)
 - rust-lang#120657 (Remove unused struct)
 - rust-lang#120661 (target: default to the medium code model on LoongArch targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@xry111
Copy link
Contributor

xry111 commented Feb 5, 2024

Hmm, why not just build std with the medium code model and leave the default normal? Then the large software should use -C code-model=medium explicitly.

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#113833 (`std::error::Error` -> Trait Implementations: lifetimes consistency improvement)
 - rust-lang#115386 (PartialEq, PartialOrd: update and synchronize handling of transitive chains)
 - rust-lang#116284 (make matching on NaN a hard error, and remove the rest of illegal_floating_point_literal_pattern)
 - rust-lang#118960 (Add LocalWaker and ContextBuilder types to core, and LocalWake trait to alloc.)
 - rust-lang#120384 (Use `<T, U>` for array/slice equality `impl`s)
 - rust-lang#120518 (riscv only supports split_debuginfo=off for now)
 - rust-lang#120657 (Remove unused struct)
 - rust-lang#120661 (target: default to the medium code model on LoongArch targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@xen0n
Copy link
Contributor Author

xen0n commented Feb 5, 2024

Hmm, why not just build std with the medium code model and leave the default normal? Then the large software should use -C code-model=medium explicitly.

That would be better but I don't know if this can be easily done (I investigated for one or two hours last night but didn't find out; maybe it is possible and it's just me forgetting), or if there would still be missing cases.

@heiher
Copy link
Contributor

heiher commented Feb 5, 2024

Hmm, why not just build std with the medium code model and leave the default normal? Then the large software should use -C code-model=medium explicitly.

That would be better but I don't know if this can be easily done (I investigated for one or two hours last night but didn't find out; maybe it is possible and it's just me forgetting), or if there would still be missing cases.

I believe using 'cargo build-std' in the current project recompiles the standard library, potentially changing its code model. But this relies on unstable features, and users might not be familiar with it. I've received feedback on default code models from users before, and if a small performance trade-off can improve the user experience, I think it's worthwhile. Thanks.

@Noratrieb
Copy link
Member

Noratrieb commented Feb 5, 2024

We could default to small and build the distributed std with medium, if you prefer that. You'd implement this logic in src/bootstrap, look at fn cargo.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 5, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#113833 (`std::error::Error` -> Trait Implementations: lifetimes consistency improvement)
 - rust-lang#115386 (PartialEq, PartialOrd: update and synchronize handling of transitive chains)
 - rust-lang#116284 (make matching on NaN a hard error, and remove the rest of illegal_floating_point_literal_pattern)
 - rust-lang#118960 (Add LocalWaker and ContextBuilder types to core, and LocalWake trait to alloc.)
 - rust-lang#120384 (Use `<T, U>` for array/slice equality `impl`s)
 - rust-lang#120518 (riscv only supports split_debuginfo=off for now)
 - rust-lang#120657 (Remove unused struct)
 - rust-lang#120661 (target: default to the medium code model on LoongArch targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dc0b1f9 into rust-lang:master Feb 5, 2024
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
Rollup merge of rust-lang#120661 - xen0n:loong-medium-cmodel, r=heiher,Nilstrieb

target: default to the medium code model on LoongArch targets

The Rust LoongArch targets have been using the default LLVM code model so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak. As [described][1] in the "Code Model" section of LoongArch ELF psABI spec v20231219, one can only make function calls as far as ±128MiB with the "normal" code model; this is insufficient for very large software containing Rust components that needs to be linked into the big text section, such as Chromium.

Because:

* we do not want to ask users to recompile std if they are to build such software,
* objects compiled with larger code models can be linked with those with smaller code models without problems, and
* the "medium" code model is comparable to the "small"/"normal" one performance-wise (same data access pattern; each function call becomes 2-insn long and indirect, but this may be relaxed back into the direct 1-insn form in a future LLVM version), but is able to perform function calls within ±128GiB,

it is better to just switch the targets to the "medium" code model, which is also "medium" in LLVM-speak.

[1]: https://github.com/loongson/la-abi-specs/blob/v2.30/laelf.adoc#code-models
@rustbot rustbot added this to the 1.78.0 milestone Feb 5, 2024
@Noratrieb
Copy link
Member

ha, it was rolled up already^^
well, if you want to change it back, feel free to make a new PR

@xry111
Copy link
Contributor

xry111 commented Feb 5, 2024

ha, it was rolled up already^^ well, if you want to change it back, feel free to make a new PR

I'm OK with this PR, now we can spend our time more efficiently by implementing call36 -> bl relaxation in Binutils and LLVM anyway :).

@xen0n xen0n deleted the loong-medium-cmodel branch February 11, 2024 12:34
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 13, 2024
…iler-errors

target: default to the medium code model on LoongArch targets

The Rust LoongArch targets have been using the default LLVM code model so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak. As described in the "Code Model" section of LoongArch ELF psABI spec v20231219 [1], one can only make function calls as far as ±128MiB with the "normal" code model; this is insufficient for very large software containing Rust components that needs to be linked into the big text section, such as Chromium.

Because:

* we do not want to ask users to recompile std if they are to build such software,
* objects compiled with larger code models can be linked with those with smaller code models without problems, and
* the "medium" code model is comparable to the "small"/"normal" one performance-wise (same data access pattern; each function call becomes 2-insn long and indirect, but this may be relaxed back into the direct 1-insn form in a future LLVM version), but is able to perform function calls within ±128GiB,

it is better to just switch the targets to the "medium" code model, which is also "medium" in LLVM-speak.

Relands [2]:  rust-lang#120661

[1]: https://github.com/loongson/la-abi-specs/blob/v2.30/laelf.adoc#code-models
[2]: rust-lang#121289 (comment)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2024
Rollup merge of rust-lang#130266 - heiher:loong-medium-cmodel, r=compiler-errors

target: default to the medium code model on LoongArch targets

The Rust LoongArch targets have been using the default LLVM code model so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak. As described in the "Code Model" section of LoongArch ELF psABI spec v20231219 [1], one can only make function calls as far as ±128MiB with the "normal" code model; this is insufficient for very large software containing Rust components that needs to be linked into the big text section, such as Chromium.

Because:

* we do not want to ask users to recompile std if they are to build such software,
* objects compiled with larger code models can be linked with those with smaller code models without problems, and
* the "medium" code model is comparable to the "small"/"normal" one performance-wise (same data access pattern; each function call becomes 2-insn long and indirect, but this may be relaxed back into the direct 1-insn form in a future LLVM version), but is able to perform function calls within ±128GiB,

it is better to just switch the targets to the "medium" code model, which is also "medium" in LLVM-speak.

Relands [2]:  rust-lang#120661

[1]: https://github.com/loongson/la-abi-specs/blob/v2.30/laelf.adoc#code-models
[2]: rust-lang#121289 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

7 participants