-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
compiler: implement labeled switch/continue #21257
Conversation
You got it! |
Looks good! Thanks for doing the work. |
4e9102f
to
edbebbc
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 we would like to minimize, but it also gets in the way of the implementation of ziglang#8220. That 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. This commit implements lowering of this new `switch_br` usage in the LLVM and C backends. The C backend just turns any case containing ranges entirely into conditionals, as before. The LLVM backend is a little smarter, and puts scalar items into the `switch` instruction, only using conditionals for the range cases (which direct to the same bb). All remaining self-hosted backends are temporarily regressed in the presence of switch range cases. This functionality will be restored for at least the x86_64 backend before merge.
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.
`.loop` is also a block, so the block_depth must be stored *after* block creation, ensuring 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.
This does *not* yet implement the new `loop_switch_br` instruction.
edbebbc
to
0d295d7
Compare
Also, don't use the special switch lowering for errors if the switch is labeled; this isn't currently supported. Related: ziglang#20627.
Brilliant work as usual, @mlugg Don't forget the release notes writeup while it's fresh in your mind :) |
Release notes are below. Labeled
|
Follow-up issue for docs: Nice demo of this in action: |
This feature was proposed in ziglang#8220, and implemented in ziglang#21257.
This feature was proposed in ziglang#8220, and implemented in ziglang#21257.
`break`ing from something which isn't a loop should always be opt-in. This was a bug in ziglang#21257.
`break`ing from something which isn't a loop should always be opt-in. This was a bug in ziglang#21257.
`break`ing from something which isn't a loop should always be opt-in. This was a bug in #21257.
The ziglings project now supports this feature https://codeberg.org/ziglings/exercises/pulls/161 |
`break`ing from something which isn't a loop should always be opt-in. This was a bug in ziglang#21257.
Add langref docs for labeled switch This feature was proposed in ziglang#8220, and implemented in ziglang#21257. Co-authored-by: Andrew Kelley <andrew@ziglang.org>
`break`ing from something which isn't a loop should always be opt-in. This was a bug in ziglang#21257.
Add langref docs for labeled switch This feature was proposed in ziglang#8220, and implemented in ziglang#21257. Co-authored-by: Andrew Kelley <andrew@ziglang.org>
Implements #8220.
This is a revival of #19812.
@Luukdegram, you might want to check the Wasm changes -- I rebased your work, but haven't tested it, so the rebase could have gone wrong somewhere, not sure. It's probably fine? (oops, the commit authorship info got lost, sorry!)
@jacobly0, I haven't implemented the new syntax in the x86_64 backend. If you'd like to do this imminently on this PR, feel free; otherwise I'll let you do it on master in your own time.
@Rexicon226, I've regressed some bits of the RISC-V backend, specifically loops and switches containing ranges. I can probably fix them up (loops certainly should be easy), but it might be easier if you can take a look?