mir: improve set construction syntax #1264
Merged
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
Instead of the generic `mnkConstr`, set constructions now use `mnkSetConstr`, encoding the semantics directly in the tree and not requiring inspection of the type. Sets, unlike the other types to which `mnkConstr` applies, is not an aggregate type, so the extra `mnkArg` wrapping is unnecessary (the operands are never consumed), saving space. Finally, `mnkRange` is introduced for representing range constructions. Previously, an `nkRange` was treated as a literal (`mnkLiteral`), which is not always correct for set constructions, where the range construction's operands can be dynamic!
* set constructions, in both constant and non-constant expression now use `mnkSetConstr` and `mnkRange` * `nkRange` is no longer considered a literal (it never really was one)
* implement support for the new nodes
* ranges are rendered as `x .. y`
* set constructions are rendered as `{...}` instead of
`construct (...)`
The new tree shapes need to be handled by all users of MIR constant expressions: * `DataTable` comparison and hashing * constant-expression to `PackedEnv` serialization * loading constant expression into VM memory * MIR tree to CGIR translation One benefit is that type inspection is no longer required for figuring out that a construction is that of a set.
In the context of temporary-elimination, set constructions are safe: the operands cannot ever overlap with the destination in memory.
For simplicity, and given that `cgirgen` is eventually going to be removed anyway, both branch labels and set construction elements share the same translation logic, even though the former only supports a subset of the latter.
Using dynamic range (range syntax with run-time operands) in set constructions crashed the compiler, due to `nkRange` always being considered a literal value.
zerbina
added
bug
saem
approved these changes
Apr 2, 2024
|
/merge |
|
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below:
|
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.
Summary
Introduce a dedicated node representation for set constructions in the
MIR, making their data layout more terse and fixing a set construction
like
{a .. b}crashing the compiler whenaorbare non-constantexpressions.
Details
mnkSetConstrandmnkRangenode kindssets are not aggregate types, wrapping the operands ofthe construction in argument nodes is unnecessary
mnkSetConstris pretty-printed as{...}instead of the genericconstruct (...)nkRangeas literal datankRangetomnkRangefor both branch labels and in setconstructions
mirgenDataTable, constant expressionserialization, and data-flow graph creation are adjusted to the new
tree shapes
mnkSetConstrandmnkRangetocnkSetConstrandcnkRangeincgirgensetElementToIr, even though this doesn't reject incorrect syntaxfor branch labels
In addition to fixing the aforementioned compiler crash, the dedicated
mnkSetConstrandmnkRangenode kinds:source of having to dispatch over the nodes' type