-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Check for usages of incomplete referred symbols in IR optimizations #5924
Labels
compiler: ir
IRgen and sway-ir including optimization passes
compiler
General compiler. Should eventually become more specific as the issue is triaged
Comments
ironcev
added
compiler
General compiler. Should eventually become more specific as the issue is triaged
compiler: ir
IRgen and sway-ir including optimization passes
labels
Apr 26, 2024
IGI-111
pushed a commit
that referenced
this issue
Apr 30, 2024
## Description This PR implements dereferencing in reassignment targets, as defined in [references](https://github.com/FuelLabs/sway-rfcs/blob/ironcev/amend-references/files/0010-references.sw). The overall effort related to references is tracked in #5063. Up to now, it was only possible to read the values references refer to. This PR allows values to be written over `&mut` references. In other words, any `*<expression that results in &mut>`is now an l-values and can be used in left-hand sides (LHS) of assignments. In most of the cases, the `<expression>` will very likely be just a reference variable, but any expression that results in `&mut` is allowed and supported. E.g.; ```Sway *mut_ref_to_u64 = 42; *if condition { &mut x } else { &mut y } = 42; *max_mut(&mut x, &mut y) = 42; ``` Additionally, the PR: - fixes #5736 by properly replacing decls in reassignment LHSs. - fixes #5737 by properly substituting types in reassignment LHSs. - fixes #5920 by properly connecting expressions in array indices to the DCA graph. - fixes #5922 by type-cheking the array indices in reassignment LHSs and forcing them to be `u64`. - improves misplaced and misleading error messages when assigning to constants and other items (see demo below). - improves error message when assigning to immutable variables by pointing to variable declaration. - improves error message when expression cannot be assigned to by pointing to the problematic part of the expression. Since Sway is more restrictive here then Rust, the restrictions, without being explained, could cause confusion. That's why special attention was given to point to the exact issue and explain what is actually supported in Sway (see demo below). - reduces number of expected warnings in references E2E tests that were previously set high because of DCA issues that are fixed in the meantime. The PR also brings additional analysis and checks to IR optimizations. Among other things, it explicitly points out the cases in which a potential usage of a symbol cannot be deterministically confirmed or denied. In this PR properly reacting for such cases is done in some optimizations. Rolling it out fully will be done in a follow up PR #5924. More advanced escape analysis will be done later on, as a part of allocating values on the heap in case of referencing. This PR implements only dereferencing in LHS. Support for referenced elements in the element based access (without dereferencing) will be done in a separate step as an ongoing work on implementing #5063. Closes #5736, #5737, #5920, #5922. ## Demo Before: ![Assignment to constant - Before](https://github.com/FuelLabs/sway/assets/4142833/e6b29d31-8e29-4ffa-acfe-a844ac50887b) ![Assignment to decl - Before](https://github.com/FuelLabs/sway/assets/4142833/ef5f025e-3dcc-4cb0-b52f-d735e2ee451e) After: ![Assignment to constant - After](https://github.com/FuelLabs/sway/assets/4142833/f3dfdbd9-2123-4a96-98a7-b3c7a3f3e7f9) ![Assignment to decl - After](https://github.com/FuelLabs/sway/assets/4142833/ae765d61-41b2-478c-96c0-d476604deec6) Expression cannot be assigned to: ![Expression cannot be assigned to](https://github.com/FuelLabs/sway/assets/4142833/384d0bb7-34a1-446a-acd5-6c8f66dc4ed5) ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
This was referenced Jun 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
compiler: ir
IRgen and sway-ir including optimization passes
compiler
General compiler. Should eventually become more specific as the issue is triaged
#5923 introduces in
memory_utils
an explicitReferredSymbols
enum that provides the information if symbols reachable from a certain value are deterministically completely identifiable, means all of the referred symbols can be identified, or if there might be symbols that are reachable from the value, but that cannot be confirmed based on the level of the analysis we currently have in IR.Introduction of this distinction immediately pointed out to the parts of IR optimizations that currently do not check if there might be referred symbols beside those returned via e.g. escape analysis.
#5923 puts
ReferredSymbols
to use in DCE optimization, but otherwise not. Other optimizations, and the escape analysis are provided via "ignorant"ReferredSymbols::any()
method that unsafely ignores potentially non-identified referred symbols.The goal of this issue is:
ReferredSymbols::any()
and on the case by case basis decide what to do in case of incompletely identified referred symbols.ReferredSymbols
) to the clients of theEscapedSymbols
. The proposal is here to makeEscapedSymbols
and enum in the same way as theReferredSymbols
.memory_utils::compute_escaped_symbols
.Load
will be done in Improve collecting of referred symbols inmemory_utils::get_symbols()
by detecting sources ofLoad
ed local raw pointers and references #6065.)The relevant parts of the code are marked with TODO comment and linked to this issue.
The text was updated successfully, but these errors were encountered: