compiler: implement labeled switch/continue #19812
Closed
Conversation
This file contains 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
|
Oh, I also need to work on SPIR-V, while it doesn't currently have CI coverage we probably don't want to regress that one. I'll take a look myself later, but in case it's non-trivial, cc @Snektron |
|
@mlugg Fixed it for the Wasm backend :) |
mlugg
force-pushed
the
computed-goto-2
branch
3 times, most recently
from
fb6a8d0
to
a3a1e6c
Compare
This commit modifies the representation of the AIR `switch_br` instruction to represent ranges in cases. Previously, Sema emitted different AIR in the case of a range, where the `else` branch of the `switch_br` contained a simple `cond_br` for each such case which did a simple range check (`x > a and x < b`). Not only does this add complexity to Sema, which -- as our secondary bottleneck -- we would like to keep as small as possible, but it also gets in the way of the implementation of ziglang#8220. This proposal turns certain `switch` statements into a looping construct, and for optimization purposes, we want to lower this to AIR fairly directly (i.e. without involving a `loop` instruction). That means we would ideally like a single instruction to represent the entire `switch` statement, so that we can dispatch back to it with a different operand as in ziglang#8220. This is not really possible to do correctly under the status quo system. For now, the actual lowering of `switch` is identical for the LLVM and C backends. This commit contains a TODO which temporarily regresseses all remaining self-hosted backends in the presence of switch case ranges. This functionality will be restored for at least the x86_64 backend before merge of this branch.
This commit introduces a new AIR instruction, `repeat`, which causes control flow to move back to the start of a given AIR loop. `loop` instructions will no longer automatically perform this operation after control flow reaches the end of the body. The motivation for making this change now was really just consistency with the upcoming implementation of ziglang#8220: it wouldn't make sense to have this feature work significantly differently. However, there were already some TODOs kicking around which wanted this feature. It's useful for two key reasons: * It allows loops over AIR instruction bodies to loop precisely until they reach a `noreturn` instruction. This allows for tail calling a few things, and avoiding a range check on each iteration of a hot path, plus gives a nice assertion that validates AIR structure a little. This is a very minor benefit, which this commit does apply to the LLVM and C backends. * It should allow for more compact ZIR and AIR to be emitted by having AstGen emit `repeat` instructions more often rather than having `continue` statements `break` to a `block` which is *followed* by a `repeat`. This is done in status quo because `repeat` instructions only ever cause the direct parent block to repeat. Now that AIR is more flexible, this flexibility can be pretty trivially extended to ZIR, and we can then emit better ZIR. This commit does not implement this. Support for this feature is currently regressed on all self-hosted native backends, including x86_64. This support will be added where necessary before this branch is merged.
The parse of `fn foo(a: switch (...) { ... })` was previously handled
incorrectly; `a` was treated as both the parameter name and a label.
The same issue exists for `for` and `while` expressions -- they should
be fixed too, and the grammar amended appropriately. This commit does
not do this: it only aims to avoid introducing regressions from labeled
switch syntax.
This commit fixes codegen for the `loop` and `switch_br` instructions in the self-hosted x86_64 backend, which was previously regressed by this branch. It does *not* yet implement the new `loop_switch_br` instruction.
`.loop` is also a block, so the block_depth being stored for the loop was incorrect. By storing the value *after* block creation, we ensure a correct block_depth to jump back to when receiving `.repeat`. This also un-regresses `switch_br` which now correctly handles ranges within cases. It supports it for both jump tables as well as regular conditional branches.
|
Any update? |
|
Looks like this didn't make it to the finish line yet. Looking forward to its revival |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
TODO: PR message.
@Luukdegram, any chance you'd be able to un-regress the WASM backend here? I broke
loopand probably fixed it (although I won't claim to have tested it), but I brokeswitch_brmore fundamentally. It now directly represents range cases, whereas Sema previously lowered these tocond_brchains. I don't know enough about WASM to easily implement this myself! If you do get around to it, note the newAir.switchIteratorto make it a little more convenient.