Skip to content

Disable jump threading UnOp::Not for non-bool #131201

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

compiler-errors
Copy link
Member

Fix #131195, where jumpthreading was optimizing !a == b into a != b for non-bool, where this is definitely not true.

@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2024

r? @wesleywiser

rustbot has assigned @wesleywiser.
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
Copy link
Member Author

I would not be surprised if jumpthreading has more type-based bugs, since we've already found two at this point.

@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 Author

#131203

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 3, 2024
…piler-errors

JumpThreading: fix bitwise not on non-booleans

Fixes rust-lang#131195

Alternative to rust-lang#131201
@compiler-errors compiler-errors added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Oct 3, 2024
@compiler-errors
Copy link
Member Author

compiler-errors commented Oct 3, 2024

This is a more conservative fix than #131203, which seems like it still has problems.

@cjgillot
Copy link
Contributor

cjgillot commented Oct 4, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 4, 2024

📌 Commit f0bfba2 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 4, 2024
@bors
Copy link
Collaborator

bors commented Oct 4, 2024

⌛ Testing commit f0bfba2 with merge 11ee3a8...

@bors
Copy link
Collaborator

bors commented Oct 4, 2024

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 11ee3a8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 4, 2024
@bors bors merged commit 11ee3a8 into rust-lang:master Oct 4, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 4, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (11ee3a8): 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 (secondary -4.2%)

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)
-4.2% [-5.3%, -3.0%] 2
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.6%)

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.6% [2.1%, 2.8%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 772.19s -> 772.334s (0.02%)
Artifact size: 342.02 MiB -> 342.01 MiB (-0.00%)

@lqd
Copy link
Member

lqd commented Oct 11, 2024

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle.

Stable nomination is also accepted in principle but it's not really needed: the new stable should have this beta backport when the stable artifacts are built in 3 days. Therefore, I'll remove the stable nomination, as it's a no-op this late in the cycle.

@rustbot label +beta-accepted -stable-nominated

@rustbot rustbot added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed stable-nominated Nominated for backporting to the compiler in the stable channel. labels Oct 11, 2024
@cuviper cuviper mentioned this pull request Oct 11, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 11, 2024
@cuviper cuviper modified the milestones: 1.83.0, 1.82.0 Oct 11, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 11, 2024
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 11, 2024
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2024
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2024
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448
- Split x86_64-msvc-ext into two jobs rust-lang#130072
- Use a small runner for msvc-ext2 job rust-lang#130151

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
…ler-errors

JumpThreading: fix bitwise not on non-booleans

Fixes rust-lang#131195

Alternative to rust-lang#131201
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
…ler-errors

JumpThreading: fix bitwise not on non-booleans

Fixes rust-lang#131195

Alternative to rust-lang#131201
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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
Development

Successfully merging this pull request may close these issues.

-Zmir-opt-level miscompiles code with panic
8 participants