Sema: Fix switch loop OPV cond lowering - #24720
Justus2308 wants to merge 2 commits into
Conversation
ca67759 to
007b0ee
Compare
|
|
007b0ee to
fa1f54a
Compare
|
|
fa1f54a to
291451a
Compare
291451a to
6b28a6c
Compare
6b28a6c to
5753e5e
Compare
|
Ok I believe I've found a much better solution to this problem now that can reuse most of the code that's already there and handles |
Special-cases runtime switches with a condition with one possible value or a singular else prong with no other prongs and lowers them to a regular `block`/`loop` instead of a `switch_br`/`loop_switch_br`.
5753e5e to
acaa075
Compare
|
I managed to get rid of the |
that's no problem, I for one appreciate when someone takes the time to get it right. |
|
I suspect this would also resolve #23973 |
| term: { | ||
| if (case_block.instructions.getLastOrNull()) |last_inst| { | ||
| if (sema.isNoReturn(last_inst.toRef())) break :term; | ||
| } | ||
| _ = try case_block.addNoOp(.unreach); | ||
| } |
There was a problem hiding this comment.
What's going on here? This should be impossible; the ZIR block, and hence the AIR block it generated, is guaranteed to end with a noreturn terminator.
| @typeInfo(Air.SwitchBr.Case).@"struct".fields.len + 2; | ||
| var cases_extra = try std.ArrayListUnmanaged(u32).initCapacity(gpa, estimated_cases_extra); | ||
| var cases_extra: std.ArrayListUnmanaged(u32) = if (single_prong) | ||
| .empty |
There was a problem hiding this comment.
| .empty | |
| undefined |
makes it clear that the arraylist is completely unused
| }; | ||
|
|
||
| assert(branch_hints.items.len == cases_len + 1); | ||
| assert(branch_hints.items.len == cases_len + 1 or (single_prong and branch_hints.items.len == 0)); |
There was a problem hiding this comment.
| assert(branch_hints.items.len == cases_len + 1 or (single_prong and branch_hints.items.len == 0)); | |
| if (!single_prong) assert(branch_hints.items.len == cases_len + 1); |
| if (else_body.len > 0) { | ||
| assert(scalar_cases_len + multi_cases_len == 0); | ||
|
|
||
| const needs_terminator = !sema.isNoReturn(else_body[else_body.len - 1].toRef()); |
4 participants
Resolves #21550
Resolves #24126
Resolves #21772
Special-cases runtime loops with a condition with one possible value and lowers them to a regular
loopinstead of aloop_switch_brconstruct. Also covers a switch loop with a single else prong, see comment below.The implementation
is basically a simplified version ofomits any value corruption safety checks or prong branch hints.analyzeSwitchRuntimeBlockand the code following it inzirSwitchBlockthat makes lots of assumptions andThis patch takes advantage of the symmetry between
loop_switch_br-switch_dispatchandloop-repeatand relies on backends being able to handle alooptargeted by multiplerepeats. This pattern should be possible but has never been emitted by Sema until now.The added tests skip the self-hosted spirv backend because it doesn't support this pattern yet.
Also fixes a typo in Liveness.