rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
93.58k stars 12.05k forks source link

Refactor Apple `target_abi` #124762

Closed madsmtm closed 1 week ago

madsmtm commented 1 week ago

This was bundled together with Arch, which complicated a few code paths and meant we had to do more string matching than necessary.

CC @BlackHoleFox as you've worked on the Apple target spec before

Related: Is there a reason why Target/TargetOptions use StaticCow for so many things, instead of an enum with defined values (and perhaps a catch-all case for custom target json files)? Tagging @Nilstrieb, as you might know?

rustbot commented 1 week ago

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

rustbot commented 1 week ago

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

lcnr commented 1 week ago

looking through this, pretty certain that this doesn't do any functional changes (so r=me on that front). Unsure whether this split is generally desirable/the state of apple abis more generally

r? compiler

Nilstrieb commented 1 week ago

lcnr says there are no functional changes, BlackHoleFox thinks this is an improvement, that sounds good. @bors r=lcnr,BlackHoleFox

bors commented 1 week ago

:pushpin: Commit 8f0d35769de3b54a5fd95ac61ce6a3ec4144f5f7 has been approved by lcnr,BlackHoleFox

It is now in the queue for this repository.

bors commented 1 week ago

:hourglass: Testing commit 8f0d35769de3b54a5fd95ac61ce6a3ec4144f5f7 with merge 2259028a70d6e1a44ad2cfd81955b577a43e8ef6...

bors commented 1 week ago

:sunny: Test successful - checks-actions Approved by: lcnr,BlackHoleFox Pushing 2259028a70d6e1a44ad2cfd81955b577a43e8ef6 to master...

rust-timer commented 1 week ago

Finished benchmarking commit (2259028a70d6e1a44ad2cfd81955b577a43e8ef6): 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) | -2.2% | [-2.2%, -2.2%] | 1 | | Improvements ✅
(secondary) | - | - | 0 | | All ❌✅ (primary) | -2.2% | [-2.2%, -2.2%] | 1 |

Cycles

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

Binary size

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

Bootstrap: 675.209s -> 673.288s (-0.28%) Artifact size: 315.95 MiB -> 316.02 MiB (0.02%)