Stabilize THIR unsafeck by matthewjasper · Pull Request #117673 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

matthewjasper

lqd, y21, JarvisCraft, Xanewok, bjorn3, Kobzol, mominul, kornelski, vacuus, wesleywiser, and 9 more reacted with hooray emoji LeSeulArtichaut and Luracasmus reacted with heart emoji

@rustbot

r? @TaKO8Ki

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

@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

Nov 7, 2023

@rust-log-analyzer

This comment has been minimized.

@lqd

A perf run would be nice as well: the compiler perf WG had plans to help with this switch because THIR unsafeck was a bit faster than MIR unsafeck.

@rustbot

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@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

Nov 7, 2023

@rust-log-analyzer

This comment has been minimized.

@matthewjasper

@rust-timer

This comment has been minimized.

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Nov 8, 2023

@bors

…ation, r=

Stabilize THIR unsafeck

Opening for a crater run in case we need a compatibility lint.

@bors

☀️ Try build successful - checks-actions
Build commit: 9ee6473 (9ee647322baa27cc6bb1a95b4595bad94b334fcf)

@rust-timer

This comment has been minimized.

@rust-timer

Finished benchmarking commit (9ee6473): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) - - 0
Improvements ✅ (primary) -0.3% [-0.8%, -0.2%] 66
Improvements ✅ (secondary) -0.8% [-1.1%, -0.4%] 9
All ❌✅ (primary) -0.3% [-0.8%, -0.2%] 66

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.7% [0.5%, 1.0%] 8
Regressions ❌ (secondary) 3.2% [1.2%, 5.2%] 2
Improvements ✅ (primary) -1.3% [-5.6%, -0.5%] 16
Improvements ✅ (secondary) -4.9% [-11.5%, -1.9%] 8
All ❌✅ (primary) -0.6% [-5.6%, 1.0%] 24

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) 2.1% [2.1%, 2.1%] 1
Improvements ✅ (primary) -0.6% [-0.9%, -0.5%] 7
Improvements ✅ (secondary) -1.3% [-2.2%, -0.8%] 4
All ❌✅ (primary) -0.6% [-0.9%, -0.5%] 7

Binary size

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.0% [0.0%, 0.0%] 6
Improvements ✅ (primary) -0.2% [-0.5%, -0.0%] 9
Improvements ✅ (secondary) -0.4% [-0.7%, -0.0%] 12
All ❌✅ (primary) -0.2% [-0.5%, -0.0%] 9

Bootstrap: 664.079s -> 663.606s (-0.07%)
Artifact size: 308.83 MiB -> 308.73 MiB (-0.03%)

@matthewjasper

@craterbot

@craterbot

🚧 Experiment pr-117673 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@lqd

@matthewjasper I also wonder if we should stabilize and enable THIR unsafeck first, and only remove MIR unsafeck after a period of time has passed, to allow for feedback and more testing? That way we'd still be able to easily flip a flag back to MIR unsafeck for beta/stable branches if we needed to.

I guess that can be discussed during FCP.

@craterbot

🚨 Report generation of pr-117673 failed: Error: missing label values ["pr-117673"]
🛠️ If the error is fixed use the retry-report command.

🆘 Can someone from the infra team check in on this? @rust-lang/infra
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@Mark-Simulacrum

@bors

📌 Commit 7832ebb has been approved by cjgillot

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-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Jan 5, 2024

@bors

@bors

This was referenced

Jan 5, 2024

@matthiaskrgr

The pr body says Removes -Zthir-unsafeck but it seems that
-Z thir-unsafeck=val -- use the THIR unsafety checker (default: yes) is still a thing, is this intended?

@compiler-errors

@matthiaskrgr: The PR description was never updated to reflect decisions made within the thread, which was just to set -Zthir-unsafeck=true by default.

@matthewjasper: It would be nice if you updated the description of this PR so it reflects what this PR actually does.

@rust-timer

Finished benchmarking commit (6bc08a7): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 0.3% [0.2%, 0.4%] 2
Improvements ✅ (primary) -0.4% [-0.9%, -0.2%] 39
Improvements ✅ (secondary) -0.6% [-1.1%, -0.4%] 9
All ❌✅ (primary) -0.4% [-0.9%, -0.2%] 39

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.6% [-8.1%, -0.6%] 12
Improvements ✅ (secondary) -3.0% [-5.2%, -1.5%] 3
All ❌✅ (primary) -2.6% [-8.1%, -0.6%] 12

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) -1.1% [-1.2%, -1.0%] 2
Improvements ✅ (secondary) -2.5% [-2.8%, -2.3%] 5
All ❌✅ (primary) -1.1% [-1.2%, -1.0%] 2

Binary size

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

Bootstrap: 669.803s -> 670.411s (0.09%)
Artifact size: 311.12 MiB -> 311.14 MiB (0.01%)

@nnethercote

Perf improvements greatly outweigh the tiny number of regressions.

@rustbot label: +perf-regression-triaged

@syvb syvb mentioned this pull request

Jan 31, 2024

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Mar 29, 2024

@he32

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Apr 2, 2024

@GuillaumeGomez

…ompiler-errors

Remove dangling .mir.stderr and .thir.stderr test files

They are not needed since rust-lang#117673

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Apr 2, 2024

@rust-timer

Rollup merge of rust-lang#123371 - eduardosm:dangling-test-files, r=compiler-errors

Remove dangling .mir.stderr and .thir.stderr test files

They are not needed since rust-lang#117673

@lcnr lcnr mentioned this pull request

Apr 5, 2024

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Oct 13, 2024

@he32

This is based on the pkgsrc-wip rust180 package, retaining the main pkgsrc changes as best as I could.

Pkgsrc changes:

Upstream chnages:

Version 1.80.1 (2024-08-08)

Version 1.80.0 (2024-07-25)

Language

Compiler

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Version 1.79.0 (2024-06-13)

Language

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Misc

Compatibility Notes

Version 1.78.0 (2024-05-02)

Language

Compiler

Target changes:

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Misc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Version 1.77.0 (2024-03-21)

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

Cargo

Rustdoc

Misc

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.