Skip to content

JumpThreading: fix bitwise not on non-booleans #131203

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 14, 2025
Merged

JumpThreading: fix bitwise not on non-booleans #131203

merged 2 commits into from
Apr 14, 2025

Conversation

clubby789
Copy link
Contributor

Fixes #131195

Alternative to #131201

@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2024

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
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 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 Oct 3, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@compiler-errors compiler-errors mentioned this pull request Oct 3, 2024
Merged
@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 3, 2024
@compiler-errors
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 3, 2024

📌 Commit fd35e82 has been approved by compiler-errors

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 Oct 3, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 3, 2024
@matthiaskrgr
Rollup merge of rust-lang#131203 - clubby789:jumpthreading-not, r=com…
1ce9d8d 
…piler-errors

JumpThreading: fix bitwise not on non-booleans

Fixes rust-lang#131195

Alternative to rust-lang#131201
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 3, 2024
Closed
@compiler-errors compiler-errors added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Oct 3, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

Ok, well, this definitely causes a crash when building [email protected]

@bors r-

Let's actually land #131201, which is a more conservative fix. Then we can work on improving the jumpthreading optimization :)

Since cjgillot authored the jump threading optimization (I think?) I can pass this off to them.

r? @cjgillot

@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 Oct 3, 2024
@clubby789
Copy link
Contributor Author

Looks like some miscompile in std 🤔 In my local testing I get println hanging on futex forever

@compiler-errors
Copy link
Member

I am very curious to learn what is wrong with this PR :D

@bors
Copy link
Collaborator

bors commented Oct 4, 2024

☔ The latest upstream changes (presumably #131201) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Apr 13, 2025

📌 Commit bee9c73 has been approved by compiler-errors

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 Apr 13, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
@bors
Auto merge of rust-lang#131203 - clubby789:jumpthreading-not, r=compi…
07e2b20 
…ler-errors

JumpThreading: fix bitwise not on non-booleans

Fixes rust-lang#131195

Alternative to rust-lang#131201
@bors
Copy link
Collaborator

bors commented Apr 13, 2025

⌛ Testing commit bee9c73 with merge 07e2b20...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented Apr 13, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 13, 2025
@rust-cloud-vms rust-cloud-vms bot force-pushed the jumpthreading-not branch from bee9c73 to 41a5d8e Compare April 13, 2025 20:30
@clubby789
Copy link
Contributor Author

Rebased and updated a test.

@bors r=compiler-errors rollup=never

@bors
Copy link
Collaborator

bors commented Apr 13, 2025

📌 Commit 41a5d8e has been approved by compiler-errors

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 Apr 13, 2025
@bors
Copy link
Collaborator

bors commented Apr 13, 2025

⌛ Testing commit 41a5d8e with merge 6e83046...

@bors
Copy link
Collaborator

bors commented Apr 14, 2025

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 6e83046 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 14, 2025
@bors bors merged commit 6e83046 into rust-lang:master Apr 14, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 14, 2025
@github-actions GitHub Actions
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 092a284 (parent) -> 6e83046 (this PR)

Test differences

No test diffs found

Job duration changes

  1. dist-aarch64-linux: 5318.0s -> 7613.4s (43.2%)
  2. dist-apple-various: 8408.9s -> 6220.8s (-26.0%)
  3. x86_64-apple-2: 4930.6s -> 4184.6s (-15.1%)
  4. x86_64-apple-1: 7984.3s -> 6994.9s (-12.4%)
  5. dist-various-2: 3418.3s -> 3209.9s (-6.1%)
  6. x86_64-msvc-ext1: 7533.8s -> 7085.1s (-6.0%)
  7. i686-gnu-2: 6685.7s -> 6370.4s (-4.7%)
  8. dist-aarch64-msvc: 8201.0s -> 8509.3s (3.8%)
  9. dist-aarch64-apple: 4389.5s -> 4532.4s (3.3%)
  10. aarch64-apple: 3719.8s -> 3603.2s (-3.1%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6e83046): 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 (primary -7.5%, secondary -2.0%)

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)		 1.5% [1.5%, 1.5%] 1
Improvements ✅
(primary)		 -7.5% [-7.5%, -7.5%] 1
Improvements ✅
(secondary)		 -3.8% [-4.3%, -3.2%] 2
All ❌✅ (primary) -7.5% [-7.5%, -7.5%] 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: 779.607s -> 779.963s (0.05%)
Artifact size: 365.51 MiB -> 365.51 MiB (0.00%)

@cjgillot cjgillot mentioned this pull request Apr 19, 2025
Merged
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 19, 2025
@ChrisDenton
Rollup merge of rust-lang#140024 - cjgillot:continue-jumping, r=compi…
9851586 
…ler-errors

Remove early exits from JumpThreading.

This removes early exits from rust-lang#131203 as I asked during review.

The correctness of the backtracking is `mutated_statement` clearing all relevant conditions. If `process_statement` fails to insert a new condition, for instance by const-eval failure, `mutated_statement`  still removes the obsolete conditions from the state.

r? `@compiler-errors`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2025
@rust-timer
Unrolled build for rust-lang#140024
a241312 
Rollup merge of rust-lang#140024 - cjgillot:continue-jumping, r=compiler-errors

Remove early exits from JumpThreading.

This removes early exits from rust-lang#131203 as I asked during review.

The correctness of the backtracking is `mutated_statement` clearing all relevant conditions. If `process_statement` fails to insert a new condition, for instance by const-eval failure, `mutated_statement`  still removes the obsolete conditions from the state.

r? `@compiler-errors`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 20, 2025
@ChrisDenton
Rollup merge of #140024 - cjgillot:continue-jumping, r=compiler-errors
a532247 
Remove early exits from JumpThreading.

This removes early exits from rust-lang/rust#131203 as I asked during review.

The correctness of the backtracking is `mutated_statement` clearing all relevant conditions. If `process_statement` fails to insert a new condition, for instance by const-eval failure, `mutated_statement`  still removes the obsolete conditions from the state.

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@cjgillot cjgillot cjgillot left review comments

@dianqk dianqk dianqk left review comments

@jieyouxu jieyouxu jieyouxu left review comments

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.

-Zmir-opt-level miscompiles code with panic