optimisation: annotations to track (reads from) mutable declarations and adapt Ir.VarE
#4637
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
to preserve inference result also tighten `VarE` subtyping and make `varE` a bit smarter
ggreif
changed the title
MutReadPrim primitiveMutReadPrim primitive
this is now an `idm` because it carries a sourcing mutability note I am also asserting that the two mechanisms are equivalent, so that in the next commit I can remove the annotation on `exp`.
ggreif
commented
Jul 29, 2024
ggreif
changed the title
MutReadPrim primitiveMutReadPrim primitive
ggreif
changed the title
MutReadPrim primitiveMutReadPrim primitive
dfx-json
approved these changes
Jul 31, 2024
crusso
reviewed
Aug 1, 2024
crusso
requested changes
Aug 1, 2024
crusso
reviewed
Aug 1, 2024
in the next commit try removing the Prim this is a review suggestiion
ggreif
commented
Aug 1, 2024
ggreif
commented
Aug 1, 2024
ggreif
commented
Aug 1, 2024
ggreif
changed the title
MutReadPrim primitiveIr.VarE
ggreif
commented
Aug 1, 2024
crusso
reviewed
Aug 2, 2024
crusso
approved these changes
Aug 2, 2024
Co-authored-by: Claudio Russo <claudio@dfinity.org>
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.
By making reads from mutable variables distinguishable, we restore the referential transparency of
VarE (Const, id). This was the reason why #4611 failed (and had to be reverted in #4623).This opens up a bunch of optimisation possibilities
VarEpath compressionLetDeliminationBlockE/NewObjEsimplificationBlockEmerging (given the defs are disjoint and outer uses don't reference inner ones; usefreevars.ml)In the long run this may allow us to extend the IR effect system with {
Load,Store} effects, and thus pave the way for transactional memory.We now have a note on
Syntax.VarE's identifier, being immutable by default. When performing type checking, this field can change to signify a mutable variable source after looking up the identifier in the environment. Then desugaring uses mutability info to create eitherIR.VarE (Const, _)(for immutable) or a`IR.VarE (Var, _) (for mutable). The IR type checker then asserts that the right kind of access is used.The Wasm output should be unchanged.
NOTE: with this PR
git revert 13d9d5748700662d30d54d6ccfcf2787deac482ecan be done and it passes. So it seems to work 😀