Support configurable memory layouts of aggregates #5286
Labels
compiler: ir
IRgen and sway-ir including optimization passes
compiler
General compiler. Should eventually become more specific as the issue is triaged
P: low
team:compiler
Compiler Team
Comments
ironcev
added
compiler
|
Related to #5227. |
7 tasks
|
Nice idea kind of cross my mind |
IGI-111
pushed a commit
that referenced
this issue
Nov 24, 2023
## Description This PR closes #5227 by refactoring access to memory layout information. The focus of the refactoring was removing the redundant code from the places which will be affected when references are introduced. Previously scattered in various places, knowledge about the memory layout of the types is now provided via (mostly) IR `Type` and (a tiny bit) IR `Constant`. Thus, the IR layer becomes the single source of truth for the memory layout. From this perspective, having other IR components like, e.g., SROA being aware of the memory layout is fine, because the memory layout assumptions are not leaking outside of the IR layer. In addition, the PR fixes two bugs: - allocation of 8x more space for local string arrays, then necessary - wrong calculation of an offset of a union variant The code outside of IR where an assumption on memory layout is still made is in the Storage API. Refactoring storage access code to remove memory layout assumptions would be a questionable effort because we anyhow want to refactor the Storage API. Therefore, a warning TODO-MEMLAY is left in the Storage API code as well as `assert`s. The `size_bytes_round_up_to_word_alignment` macro is still used in exactly four places, where an alignment is needed, but not of a type size, then rather some other structure in memory like, e.g., locals on the stack. The PR also fully document memory layout decisions and asserts them with unit test. The consumers of the memory layout information like, e.g., `DataSection` of course still need to be aware of the meaning of the layout information and use it according to their own use case. E.g.: - the `DataSection` places constants according to the expected layout - `asm_generation` `functions` module places local variables on the stack - etc The potential next steps are supporting more optimal layout of aggregates in terms of memory usage, or even providing configurable memory layouts. For this, there is a separate issue #5286. Closes #5227. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [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. --------- Co-authored-by: xunilrj <xunilrj@hotmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The memory layout of Sway types is hardcoded at the moment and tested and documented in irtype.rs. In the case of structs (means also tuples and enums which are the structs of tags and unions) whose fields are smaller then the word, the default padding is not optimal in the sense of memory space usage.
We can think of better packing of structs, again hardcoded, but even go a step further and provide an equivalent to #[repr(...)] in Rust.
The text was updated successfully, but these errors were encountered: