Skip to content
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

IR explicit pointer refactor. #4336

Merged
merged 24 commits into from
Apr 2, 2023
Merged

Conversation

otrho
Copy link
Contributor

@otrho otrho commented Mar 24, 2023

This is a big refactor of the IR and ASMgen, closes #2819.

The main purpose is multi-faceted but still generally related to pointers in IR while supporting various backends other than just Fuel VM.

  • Pointer is a member of Type (again).
  • Instructions for manipulating aggregates such as extract_value and insert_element have been removed.
  • A get_elem_ptr (GEP) instruction is now used to index aggregate fields.
  • addr_of has been replaced by ptr_to_int.

Sway does not have explicit references. But values which cannot fit in a register must be passed by reference. Therefore the implementation has generally followed the idea of 'copy' types and 'reference' types. The size of a register is target specific, but until recently the only target we had was the Fuel VM. This is no longer true so assumptions about 'copy' types vs 'reference' types needed to be relaxed.

The IRgen now tries not to assume which atomic types are 'reference' types and assumes all atomic types (including b256) may be passed by value. Aggregate types (structs, enums, arrays) are all assumed to be passed by reference. All memory reads and writes are performed by load or store rather than explicitly using mem_copy.

The IR itself makes no assumptions about passing by value or reference as all uses of pointers are explicit. If a value is to be passed by reference then is must use a pointer type.

To accommodate the 64-bit register size of the Fuel VM and atomic types which don't fit (primarily b256, but potentially any integer type wider than 64 bits) some new transformational 'demotion' passes have been introduced. These find all the unsupported uses of wide types and 'demotes' them to temporary values on the stack and refers to their pointers.

  • constdemotion - demotes large immediate constants.
  • argdemotion - demotes large arguments to functions, explicitly passing them by reference.
  • retdemotion - demotes large return values to be stored locally by the caller and updated in place by the callee.
  • miscdemotion - demotes other miscellaneous values:
    • arguments to the log instruction.
    • arguments to ASM blocks.
    • arguments to the ptr_to_int instruction.

These passes don't alter the use by value vs reference semantics though and will still use load and store on the large values. To fix this another new pass has been introduced. memcpyopt will primarily search for stores of loads and convert them to a mem_copy_val. This greatly simplifies ASMgen, which can now assume a load or store is only for register sized values, and MCP can be used otherwise.

Unfortunately adding these temporaries adds some bloat to the raw IRgen output. memcpyopt also attempts copy propagation to reduce redundant memory copies. At this stage it uses a very conservative algorithm but may be improved in the future when we can introduce comprehensive data flow analysis.

This is still a partial work in progress in that it has uncovered many new issues which need to be addressed, which I'll be adding shortly. They should be listed below as they'll reference this PR.

@otrho otrho requested a review from a team March 27, 2023 03:28
@otrho otrho added big this task is hard and will take a while code quality compiler: ir IRgen and sway-ir including optimization passes compiler: codegen Everything to do with IR->ASM, register allocation, etc. compiler: optimization IR Optimization Passes labels Mar 27, 2023
@otrho otrho marked this pull request as ready for review March 27, 2023 03:48
Copy link
Contributor

@vaivaswatha vaivaswatha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for ir_generation/function.rs and the tests, I've (mostly) reviewed the other files. Leaving some comments before I get to these two.

sway-ir/src/irtype.rs Outdated Show resolved Hide resolved
sway-ir/src/verify.rs Outdated Show resolved Hide resolved
sway-ir/src/instruction.rs Show resolved Hide resolved
sway-ir/src/optimize/arg_demotion.rs Outdated Show resolved Hide resolved
sway-ir/src/optimize/arg_demotion.rs Outdated Show resolved Hide resolved
sway-ir/src/optimize/arg_demotion.rs Outdated Show resolved Hide resolved
sway-ir/src/optimize/const_demotion.rs Outdated Show resolved Hide resolved
vaivaswatha
vaivaswatha previously approved these changes Mar 29, 2023
@otrho otrho requested a review from a team March 29, 2023 11:15
@otrho otrho enabled auto-merge (squash) March 29, 2023 11:15
@otrho otrho merged commit d6238ae into master Apr 2, 2023
@otrho otrho deleted the otrho/2819_pointer-as-an-ir-type branch April 2, 2023 21:30
vaivaswatha added a commit that referenced this pull request Jul 19, 2023
This was required previously, but after the IR refactoring (#4336),
this is no longer required, and actually worsens things.

Fixes #4747. For the two tests mentioned there,
(both based on https://github.com/hashcloak/fuel-crypto/)
the compile time now comes down from 30m -> 16s and 180m -> 31s.
vaivaswatha added a commit that referenced this pull request Jul 21, 2023
This was required previously, but after the IR refactoring (#4336), this
is no longer required, and actually worsens things.

Fixes #4747. For the two tests mentioned there, (both based on
https://github.com/hashcloak/fuel-crypto/) the compile time now comes
down from
1. 30m -> 16s
2. 180m -> 31s

Also fixes two bugs uncovered:
1. Bug in DCE - Fixes #4763.
(1b512b8)
2. Bug in interaction b/w asm gen and reg alloc spiller
(7a40496).

---------

Co-authored-by: IGI-111 <igi-111@protonmail.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big this task is hard and will take a while code quality compiler: codegen Everything to do with IR->ASM, register allocation, etc. compiler: ir IRgen and sway-ir including optimization passes compiler: optimization IR Optimization Passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aggregates and memory management in the IR is generally broken.
3 participants