-
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
Implement IR v2.0 #6351
Labels
compiler: codegen
Everything to do with IR->ASM, register allocation, etc.
compiler: ir
IRgen and sway-ir including optimization passes
compiler
General compiler. Should eventually become more specific as the issue is triaged
team:compiler
Compiler Team
Comments
8 tasks
ironcev
added a commit
that referenced
this issue
Sep 4, 2024
## Description This PR implements the `__contract_ret` as a diverging (returning `!`) and accordingly the FuelVM `retd` instruction as being a terminator. This strict semantics is needed in the #6351 which introduces strict(er) verification of IR invariants. ## 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). - [ ] 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.
8 tasks
ironcev
added a commit
that referenced
this issue
Sep 14, 2024
…ges (#6545) ## Description This PR adds additional tests for constants. Those test cases are important for the implementation of #6351 which will completely restructure compilation of constants. Tests for recursive `const` definitions should also be considered before we start implementing `const fn` and `const trait`. In particular, we want to better track and provide precise error messages in case of unintended attempt to recursively define a constant in a complex situation that includes `const fn` and `const trait`. Related to #6534, #6537, #6538, #6539, #6540, #6543, and #6544. ## Checklist - [x] I have linked to any relevant issues. - [ ] 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). - [ ] 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.
vaivaswatha
added a commit
that referenced
this issue
Feb 20, 2025
First step as part of #6351 . --------- Co-authored-by: IGI-111 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
compiler: codegen
Everything to do with IR->ASM, register allocation, etc.
compiler: ir
IRgen and sway-ir including optimization passes
compiler
General compiler. Should eventually become more specific as the issue is triaged
team:compiler
Compiler Team
Motivation
Our current IR implementation has several hindering issues, most of them coming from how we deal with pointers and aggregates in the IR. The purpose of this tracking issue is to explain the current problems and their impact, list the agreed solution steps, and track their implementation.
Terminology
To disambiguate the term pointer, in the remaining part of this description we will consistently use the following terminology:
Pointer
for the IRTypeContent::Pointer
raw_ptr
type&T
typeCurrent Issues
Pointer
is conceptually seen as an equivalent for an aggregate type. It is assumed that whenever we encounter aPointer
in IR, it represents an aggregate. This assumption makes it impossible to formulate references and raw pointers asPointer
s. (They are currently modeled asu64
which leads to various other issues explained below.) It also represents a semantic issue, becausePointer
s are not treated as values. E.g., in thecompile_expression_to_value
every pointer will be turned into aload
, assuming that thePointer
itself cannot be a value, and that the value must be behind thePointer
.ptr_to_int
does not check if the input argument is aPointer
. Anything can be turned intou64
and, later on, turned back into aPointer
by usingint_to_ptr
. This is a severe issue that led to a serious misuse, in which we pairptr_to_int
+int_to_ptr
to obtainPointer
s to SSA registers! Having "Pointers
" to SSA registers is semantically wrong and makes the generated IR being semantically wrong for the most of the optimization pipeline, until it reaches demotion passes, which promote those SSA registers into memory making the previously obtainedPointer
s valid. Having semantically invalid IR all the way down to the final demotion steps is an issue in itself, but it also harms optimizations that usually bail out when encounteringptr_to_int
instructions.get_elem_ptr
since we don't have LLVM equivalents ofinsert/extractelement
(which would allow us to manipulate aggregates in SSA registers). This restriction on its own is not an issue. The issue is that we generate IR where aggregates are stored in SSA registers, and then use theptr_to_int
+int_to_ptr
pairs described above to obtain aPointer
to SSA register (not memory!) containing the aggregate. The aggregate is then manipulated viaget_elem_ptr
as if it is in memory. Essentially, we have a semantically wrong mixture ofget_elem_ptr
andstore
andload
attempting to simulateinsert/extractvalue
.ptr_to_int
+int_to_ptr
pairs on an SSA function argument value (!!) generating essentially a semantically wrong IR, that gets "fixed" after thearg-demotion
pass.u64
s and notPointer
s. This causes several issues. The use ofptr_to_int
immediately marks pointees as escaping and hinders possible optimizations, because they do not necessarily need to escape the function scope. It also makes escape analysis difficult and in a general case impossible. E.g., everyu64
argument or return value might actually be a pointer, and the only way to figure that out would be to check the usages. In a general case, even the local usage must not reveal that au64
value passed around is actually a pointer. E.g., if araw_ptr
or a&T
is passed as an argument and just forwarded to other functions, it will not be possible to spot, by just looking at the current function IR, that thoseu64
arguments actually represent pointers. Having pointers represented asu64
also makes writing optimizations harder to reason about and more error-prone. Current situation in IR optimizations is a paradoxical one. We strip the information about pointers away, by turning them intou64
, and then try to recognize certain IR code patterns that would reveal to us, that thoseu64
s are actually pointers.const-demotion
pass. This makes it impossible to take pointers on global constants in a semantically correct way. It also generates semantically invalid IR if the constants are aggregates whose elements are accessed. When obtaining a pointer to a global constant, we should always get the same address, no matter in which function the pointer is taken. The way we currently deal with global constants, and with constants in general, breaks this semantics. Although we can add global constants to aModule
viaadd_global_constant
, those constants are, later on, not retrieved in IR asPointer
s to global constants but rather asValue
s ofValueDatum::Constant
. This essentially causes issues conceptually the same as those explained in the above points. Until we reach theconst-demotion
pass, all constants are SSA values.target_fuel
module.Agreed Solution
We were discussing several possibilities to solve the above issues. E.g., to introduce
insert/extract_value
instructions and shift their lowering to ASM, etc. The goal was to come up with an always consistent IR code that is simple to reason about, while being pragmatic when it comes to implementation costs of the IR restructuring.We've decided to do the following:
get_elem_ptr
. But the aggregates will be guaranteed to be in memory in order to be manipulated.mem2reg
will turn them into SSA values, as it is now. This will ensure that all aggregates consistently stay in memory until they reach the SROA pass (or promotion passes which we can add in the future). Essentially, we are moving from having aggregates in registers and demoting them, to having them in memory and promoting them.ptr_to_int
is used only onPointer
s. This will forbid any misuse. Also, once we represent pointers asPointer
s (see below), we do not expect to haveptr_to_int
to be used so prominently at all.Pointer
s. It will be possible to take__addr_of
on an arbitrary type, and not only an aggregates as it is now.get_global_const
will be introduced to access global constants. It will return aPointer
to the constant similarly toget_local
andget_config
. A promotion pass can still promote a global constant if there are no pointers taken to it, and if the promotion is valuable (e.g., having a constant that fits into an immediate value).The impact of the planned changes on the IR is significant, both in the semantics and implementation, and spread throughout the whole IR code base. This, IMO, justifies "branding" the planned changes as IR v2.0. Better code-names are welcome 😄
The text was updated successfully, but these errors were encountered: