mir: remove the tree delimiter nodes #1334
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
It has the same role as the `cnkBinding` node in the CGIR, that is, to group a field name and value expression in an object construction expression together. Previously, the `mnkField` and expression nodes loosely followed each other, making `mnkObjConstr` subtrees heterogenous, which complicates some traversal (or at least it will, once more traversal is tree- length-based).
`mnkPathNamed`, `mnkPathVariant`, `mnkPathPosition`, and `mnkName` are all going to change shape. For better forward compatibility, dedicated construction template for them are added and used. The `ekNone` effect kind is introduced for being able to signal "no effect" when constructing a `mnkName` tree.
Instead of requiring delimiter nodes, subtree root nodes now store a their number of subnodes. Multiple subtree root nodes previously stored extra information, which is now no longer possible. Most tree traversal routines are updated accordingly, and those specific to `mnkEnd` nodes are removed. Since it's obsolete now, the `GeneralEffect` enum type is removed.
* skipping end nodes is unnecessary and wrong now * instead of iterating until an `mnkEnd` node is found, a for loop using the node count suffices
A `MirBuffer` instance keeps the track of the number of the subnodes in the currently constructed tree, updating the `len` field in the root node on subtree completion. The value is saved on the stack when starting a nested subtree. For manual subtree construction (i.e., not using the `subTree` template), the `start` and `finish` procedures are provided. Since the builder manages updating the length field, manually providing the number of nodes is unnecessary (`mirgen` is updated accordingly).
Subtree nodes no longer being able to store non-length information requires the information to be stored elsewhere. The `mnkImmediate` node is added, and used as an ad-hoc way of storing information in the tree. * call trees store whether the call modifies some global state in an immediate value in the first slot * `mnkName` trees store the tag as an immediate value in the first subnode -- `mnkTag` nodes are removed * the named-field-access trees store the field position in an `mnkField` node in the second slot * `mnkPathPos` stores the position as an immediate value in the second slot The tree construction throught the MIR processing is updated accordingly.
It replaces the now-unavailable `findEnd` + `previous` combination.
* inspection of call, path, and `mnkName` is adjusted to work with the new tree shapes * dedicated query procedures are used instead of manual indexing where sensible
There's no more end node that needs to be patched.
zerbina
added
refactor
Manually constructing a `mnkCall`/`mnkCheckedCall` sub-tree is prone to mistakes, and it also complicates future changes to the syntax (should any be made). `mirconstr` now provides the `rawBuildCall` template for building call trees where full control over the makeup is needed.
|
Using a |
|
FYI: minor updates to PR description (typo/grammar) |
saem
approved these changes
Jun 10, 2024
Co-authored-by: Saem Ghani <[email protected]>
|
Thank you for the review, @saem. |
|
/merge |
|
Merge requested by: @zerbina 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
Instead of marking the end of a subtree with a dedicated node, the
number of child nodes in a subtree is now stored in the subtree's root
node. The idea is to:
Details
The idea of using dedicated end nodes originated at a time where:
Neither is the case anymore, and replacing the delimiter
mnkEndnodeswith storing the length has some benefits:
MirTree(reduced memory usage, fasterscanning, faster copying, etc.)
O(1)length lookupsingle child node
ignored/considered anymore
The downside of removing the delimiters nodes are that:
needs to be kept track of
cost
Ultimately, the upsides outweigh the downsides, especially since
walking the tree upwards is seldomly done.
Adjustments
For memory efficiency, some subtree root nodes (like
mnkPathPos)stored extra information in the root node. Since all subtree root nodes
now need to store the length, this is no longer possible, and the
information has to be moved into separate nodes:
immediate value (using the new
mnkImmediatenode) in the tree'sfirst slot
tree's second slot
For easier processing of
mnkObjConstr/mnkRefConstrtrees, themnkBindingis introduced, which groups themnkFieldand associatedexpression together in a subtree.
The rest of the changes are about:
MirBuilderroutines to track the child node count andpatch the root nodes
layout