Skip to content

Subtree sync for rustc_codegen_cranelift #135125

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

Merged
merged 46 commits into from
Jan 5, 2025

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jan 5, 2025

Aside from a Cranelift update, nothing major this time.

r? @ghost

@rustbot label +A-codegen +A-cranelift +T-compiler

bjorn3 and others added 30 commits December 6, 2024 12:10
As a rule, the application of `unsafe` to a declaration requires that use-sites
of that declaration also require `unsafe`. For example, a field declared
`unsafe` may only be read in the lexical context of an `unsafe` block.

For nearly all safe traits, the safety obligations of fields are explicitly
discharged when they are mentioned in method definitions. For example,
idiomatically implementing `Clone` (a safe trait) for a type with unsafe fields
will require `unsafe` to clone those fields.

Prior to this commit, `Copy` violated this rule. The trait is marked safe, and
although it has no explicit methods, its implementation permits reads of `Self`.

This commit resolves this by making `Copy` conditionally safe to implement. It
remains safe to implement for ADTs without unsafe fields, but unsafe to
implement for ADTs with unsafe fields.

Tracking: rust-lang#132922
A bunch of cleanups

These are all extracted from a branch I have to get rid of driver queries. Most of the commits are not directly necessary for this, but were found in the process of implementing the removal of driver queries.

Previous PR: rust-lang#132410
…iler-errors

Make `Copy` unsafe to implement for ADTs with `unsafe` fields

As a rule, the application of `unsafe` to a declaration requires that use-sites of that declaration also entail `unsafe`. For example, a field declared `unsafe` may only be read in the lexical context of an `unsafe` block.

For nearly all safe traits, the safety obligations of fields are explicitly discharged when they are mentioned in method definitions. For example, idiomatically implementing `Clone` (a safe trait) for a type with unsafe fields will require `unsafe` to clone those fields.

Prior to this commit, `Copy` violated this rule. The trait is marked safe, and although it has no explicit methods, its implementation permits reads of `Self`.

This commit resolves this by making `Copy` conditionally safe to implement. It remains safe to implement for ADTs without unsafe fields, but unsafe to implement for ADTs with unsafe fields.

Tracking: rust-lang#132922

r? ```@compiler-errors```
…e can be invalid to toggle

Also rename some things for extra clarity
It is effectively a global resource and the jobserver::Client in Session
was a clone of GLOBAL_CLIENT anyway.
It is treated as a map already. This is using FxIndexMap rather than
UnordMap because the latter doesn't provide an api to pick a single
value iff all values are equal, which each_linked_rlib depends on.
A bunch of cleanups (part 2)

Just like rust-lang#133567 these were all found while looking at the respective code, but are not blocking any other changes I want to make in the short term.
`rustc_span::symbol` defines some things that are re-exported from
`rustc_span`, such as `Symbol` and `sym`. But it doesn't re-export some
closely related things such as `Ident` and `kw`. So you can do `use
rustc_span::{Symbol, sym}` but you have to do `use
rustc_span::symbol::{Ident, kw}`, which is inconsistent for no good
reason.

This commit re-exports `Ident`, `kw`, and `MacroRulesNormalizedIdent`,
and changes many `rustc_span::symbol::` qualifiers in `compiler/` to
`rustc_span::`. This is a 200+ net line of code reduction, mostly
because many files with two `use rustc_span` items can be reduced to
one.
Signed-off-by: acceptacross <csqcqs@gmail.com>
Cranelift will return None from TrapCode::user(0).

Fixes rust-lang/rustc_codegen_cranelift#1548
Wine started crashing for whatever reason. Supporting native Windows is
much more important and already tested separately.
Rather than trying to fish it out of the default target location dist/.
This reduces the amount of places where the presence of the dist dir is
hardcoded.
They can still be set using HOST_TRIPLE and TARGET_TRIPLE.
this was easier than expected. here is an example of using RUSTC_LOG with a build of cranelift from rust-lang/rust:
```
$ RUSTC_LOG=rustc_codegen_cranelift cargo +stage1 b
   Compiling example v0.1.0 (/home/jyn/src/example)
 INFO rustc_codegen_cranelift codegen crate example
 INFO rustc_codegen_cranelift codegen crate example
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.44s
```
Variants::Single: do not use invalid VariantIdx for uninhabited enums

~~Stacked on top of rust-lang#133681, only the last commit is new.~~

Currently, `Variants::Single` for an empty enum contains a `VariantIdx` of 0; looking that up in the enum variant list will ICE. That's quite confusing. So let's fix that by adding a new `Variants::Empty` case for types that have 0 variants.

try-job: i686-msvc
hook up tracing to cg_cranelift
Cranelift now uses the same section name for all subsections which
reduces the size overhead of -ffunction-sections.
It isn't all that useful and wine has started crashing in CI
@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. has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 5, 2025

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/rust.git master
$ git push --force-with-lease

The following commits are merge commits:

@rustbot rustbot added A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend labels Jan 5, 2025
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 5, 2025

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

@@ -195,6 +195,7 @@ const EXCEPTIONS_CRANELIFT: ExceptionList = &[
("cranelift-module", "Apache-2.0 WITH LLVM-exception"),
("cranelift-native", "Apache-2.0 WITH LLVM-exception"),
("cranelift-object", "Apache-2.0 WITH LLVM-exception"),
("foldhash", "Zlib"),
Copy link
Member Author

Choose a reason for hiding this comment

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

The same exception exists for rustc.

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 5, 2025

@bors r+ p=1 subtree sync

@bors
Copy link
Collaborator

bors commented Jan 5, 2025

📌 Commit 8e51a89 has been approved by bjorn3

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. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 5, 2025
@bors
Copy link
Collaborator

bors commented Jan 5, 2025

⌛ Testing commit 8e51a89 with merge 3323bbe...

@bors
Copy link
Collaborator

bors commented Jan 5, 2025

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing 3323bbe to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 5, 2025
@bors bors merged commit 3323bbe into rust-lang:master Jan 5, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 5, 2025
@bjorn3 bjorn3 deleted the sync_cg_clif-2025-01-05 branch January 5, 2025 19:42
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3323bbe): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.4% [-0.4%, -0.4%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 1.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.4% [1.4%, 1.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.4% [1.4%, 1.4%] 1

Cycles

Results (secondary -2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.1% [-2.2%, -2.1%] 2
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 764.981s -> 762.79s (-0.29%)
Artifact size: 325.62 MiB -> 325.66 MiB (0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend A-testsuite Area: The testsuite used to check the correctness of rustc has-merge-commits PR has merge commits, merge with caution. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.