builder: Emit a plain jump for a switch with no cases#891
Open
Dnreikronos wants to merge 3 commits into
Open
Conversation
This comment has been minimized.
This comment has been minimized.
Dnreikronos
force-pushed
the
fix-empty-switch-no-cases
branch
from
288ecb5 to
a41879d
Compare
A `SwitchInt` with only an `otherwise` target reaches the backend as a switch with an empty case list. `gcc_jit_block_end_with_switch` requires an integer discriminant, so it rejects the `bool` discriminant produced by a range-pattern comparison under `-Zmir-preserve-ub`, which keeps the otherwise-simplified switch instead of lowering it to a `goto`. Such a switch is equivalent to an unconditional jump to the default block, so emit that instead.
Dnreikronos
force-pushed
the
fix-empty-switch-no-cases
branch
from
a41879d to
d865ac7
Compare
Contributor
|
Thanks for your work. I believe we might be able to add the test the following way. and add a commented line that does something like |
Unlike the compile-time TEST_FLAGS, this is read at run time so a single test can opt into flags such as -Zmir-preserve-ub through an ignore-if directive that checks whether the variable is set.
A range-pattern match compiled with -Zmir-preserve-ub leaves a SwitchInt with no cases whose discriminant is a bool comparison result. The test is skipped unless CARGO_TEST_FLAGS is set so it only runs when that flag is passed.
Author
Just adjusted :) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Closes #881
A range-pattern match compiled with
-Zmir-preserve-ubcan leave aSwitchIntwhose only target isotherwise. That reaches the backend as a switch with an empty case list, and in the reproducer the discriminant is theboolresult of the range comparison.gcc_jit_block_end_with_switchonly accepts an integer discriminant, so it errors out and leaves the block unterminated.A switch with no cases always goes to the default block, so emit a plain jump in that case rather than building a degenerate switch. This is also what
codegen_ssaalready relies on for the LLVM backend, which tolerates the empty switch.Verification
Built the backend locally and ran the exact reproducer from the issue with
-Zmir-preserve-ub -Clink-dead-code=true:Without this change, it reproduces the reported error verbatim:
With this change, it compiles cleanly and the resulting binary runs (exit 0).
The MIR confirms the shape.
bb0holdsswitchInt(_2: bool) -> [0: bb2, otherwise: bb1], which has a single case and is lowered to a conditional branch bycodegen_ssa, so it never reaches here.bb1holdsswitchInt(_3: bool) -> bb2, a switch with zero cases, which is the one that hit the empty-case path.No in-tree regression test, since the
lang_testerharness has no per-file compile-flags and the bug needs-Zmir-preserve-ub. Happy to add one if there's a preferred spot.