tests: basic-[debugger-]stepping.rs: Disable SingleUseConsts MIR pass temporarily #147426
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
To make more of the test pass. It should not be necessary to disable this pass, but by doing it we can avoid further regressions while we figure out how to solve this use case properly. The last two `FIXME(33013)` didn't go away from this change, only the first six. But that can be investigated separately.
rustbot
added
S-waiting-on-review
Collaborator
|
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
Enselic
force-pushed
the
better-stepping
branch
from
b0564e3 to
5b47948
Compare
This was referenced Oct 7, 2025
Closed
Member
|
r? @saethlin I'm personally dubious on disabling MIR passes in debuginfo tests... but I don't have the context here to really evaluate the tradeoffs. |
Member
Author
|
If we don't disable the pass, we can get other accidental regressions. If we disable the pass we at least know the situation will not get worse. And that risk of additional regressions is high since it already happened at least twice in the past. Long term we should do a proper fix of course. Amendment 2025-10-15 to avoid pinging people:You are right. We need to test both with and without SingleUseConsts to meaningfully prevent regressions. For that we need support for revisions, which I will look into adding. #147799 is the first step. |
Member
|
I'm very confused by the way you are describing this. If there are actual regressions happening in the behavior of the compiler, then what this PR does doesn't fix them. All it does is make sure the test suite keeps passing, which would make it easier for us to ship a regression. |
saethlin
added
S-waiting-on-author
Collaborator
|
☔ The latest upstream changes (presumably #146414) made this pull request unmergeable. Please resolve the merge conflicts. |
1 task
bors
added a commit
that referenced
this pull request
Nov 14, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in #144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing #33013. CC #147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
bors
added a commit
that referenced
this pull request
Nov 14, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in #144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing #33013. CC #147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Nov 17, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in rust-lang/rust#144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae92dc76d9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing rust-lang/rust#33013. CC rust-lang/rust#147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To make more of the test pass. It should not be necessary to disable this pass, but by doing it we can avoid further regressions while we figure out how to solve this use case properly.
We know this use case is sensitive to regressions because it already happened at least once. See #33013 (comment).
CC #130896
The last two
FIXME(#33013)go away with #147462 in a completely different way (there will be trivial conflicts for me to resolve in basic-stepping.rs when one of them lands, but that's fine.)