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

Rollup of 5 pull requests #111984

Merged
merged 19 commits into from
May 26, 2023
Merged

Rollup of 5 pull requests #111984

merged 19 commits into from
May 26, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nnethercote and others added 19 commits May 24, 2023 10:05
It is small and has a single call site, and this change will facilitate
the next commit.
Three of the four methods in `DefaultPartitioning` are defined in
`default.rs`. But `merge_codegen_units` is defined in a separate module,
`merging`, even though it's less than 100 lines of code and roughly the
same size as the other three methods. (Also, the `merging` module
currently sits alongside `default`, when it should be a submodule of
`default`, adding to the confusion.)

In rust-lang#74275 this explanation was given:

> I pulled this out into a separate module since it seemed like we might
> want a few different merge algorithms to choose from.

But in the three years since there have been no additional merging
algorithms, and there is no mechanism for choosing between different
merging algorithms. (There is a mechanism,
`-Zcgu-partitioning-strategy`, for choosing between different
partitioning strategies, but the merging algorithm is just one piece of
a partitioning strategy.)

This commit merges `merging` into `default`, making the code easier to
navigate and read.
I find that these structs obfuscate the code. Removing them and just
passing the individual fields around makes the `Partition` method
signatures a little longer, but makes the data flow much clearer. E.g.

- `codegen_units` is mutable all the way through.
- `codegen_units`'s length is changed by `merge_codegen_units`, but only
  the individual elements are changed by `place_inlined_mono_items` and
  `internalize_symbols`.
- `roots`, `internalization_candidates`, and `mono_item_placements` are
  all immutable after creation, and all used by just one of the four
  methods.
This is something that took me some time to figure out.
Issue rust-lang#106021
Apple LD requires build versions in object files for Catalyst
…henkov

Fix linking Mac Catalyst by including LC_BUILD_VERSION in object files

Hello. My first rustc PR!

Issue rust-lang#106021 prevents Rust code from being linked into Mac Catalyst applications. Apple's LD has started requiring object files to contain version information about the platform they were built for, such as:
* the "deployment target" (minimum supported OS version),
* the SDK version
* the type of the platform (macOS/iOS/catalyst/tvOS/watchOS all have a different number).

This is currently only enforced when building for Mac Catalyst.

Rust uses the `object` crate which added support for including this information starting with `0.31.0`. ~~I upgraded it along with `thorin-dwp` so that everything depends on 0.31.
Apparently 0.31 [pulls in](gimli-rs/object#463) `ruzstd` due to a [new ELF standard](https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections) because its `compression` feature is enabled by thorin. If you find this objectionable, let me know what the best way to avoid pulling in those dependencies might be.~~

**(`object` upgraded in rust-lang#111413

I then added two commits:
* The first one adds very basic, hard-coded support for calling `set_macho_build_version` for `-macabi` (Catalyst) targets, where it claims deployment target of Catalyst 14.0 and SDK of 16.2.
* The second weaves the versioning through `rust_target::spec::TargetOptions`, so that we can stick to specifying all target-related info in one place.

Kudos to ``@ara4n`` for writing [this gist](https://gist.github.com/ara4n/320a53ea768aba51afad4c9ed2168536).
…wiser

CGU cleanups

Some code clarity improvements I found when reading this code closely.

r? ``@wesleywiser``
…thomcc

Clarify safety concern of `io::Read::read` is only relevant in unsafe code

We have this clarification note in other similar place like [Iterator::size_hint](https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.size_hint).

The lack of clarification might lead to confusion to Rust beginners. [Relevant URLO post](https://users.rust-lang.org/t/can-read-overflow-a-buffer/94347).
Add test for RPIT defined with different hidden types with different substs

Close rust-lang#111943
Correct comment on privately uninhabited pattern.

Follow-up to rust-lang#111624 (comment)

r? `@Nadrieril`
@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels May 26, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented May 26, 2023

📌 Commit dd74ae0 has been approved by matthiaskrgr

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 May 26, 2023
@bors
Copy link
Contributor

bors commented May 26, 2023

⌛ Testing commit dd74ae0 with merge 1221e43...

@bors
Copy link
Contributor

bors commented May 26, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 1221e43 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 26, 2023
@bors bors merged commit 1221e43 into rust-lang:master May 26, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 26, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#111951 efb53129a862b03dae08cc4d47c177c7242bf53b
#111947 684aaecec6264ea7f0b6e12b482b3b60c4247b47
#111940 ab09657d9917ed3a9ccb9d118ff85ba630ab5673
#111899 7eef7ac9b7022f2fa4da432dd542449b86a4c2e8
#111384 bfd68dc9fd6a42dff019e1dfc92e87e6114fb465

previous master: be72f2587c

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1221e43): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

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)
-1.1% [-1.1%, -1.1%] 1
All ❌✅ (primary) - - 0

Cycles

Results

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)
-5.9% [-7.2%, -2.4%] 7
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 643.81s -> 644.999s (0.18%)

@matthiaskrgr matthiaskrgr deleted the rollup-6u7ynyv branch March 16, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants