Skip to content

feat[venom]: merge memory writes#4341

Merged
charles-cooper merged 157 commits intovyperlang:masterfrom
HodanPlodky:feat/memmerging
Dec 18, 2024
Merged

feat[venom]: merge memory writes#4341
charles-cooper merged 157 commits intovyperlang:masterfrom
HodanPlodky:feat/memmerging

Conversation

@HodanPlodky
Copy link
Copy Markdown
Collaborator

@HodanPlodky HodanPlodky commented Oct 31, 2024

What I did

Reimplementation of methods from original pipeline optimizer into the venom pipeline, that handled merging of the multiple memory operations that writes into the continuous chuck of the memory into one instruction that does in batch.

How I did it

Created new pass for this purpose

How to verify it

I have written new tests

Commit message

Adapt the memory merging pass from the legacy pipeline into venom
pipeline with additional improvements. It merges instructions that copy
data or zero memory into a single copy instruction if possible (with the
exception that if the copy is exactly 32 bytes, it uses a load/mstore
pair to save 1 byte / 3 gas). It also handles sequences of interleaved
and out-of-order copies, even though these are not currently generated
by the frontend.

Examples:

input1:
```
    %1 = mload 0
    mstore 1000, %1
    %2 = mload 32
    mstore 1032, %2
```
output1:
```
    mcopy 1000, 0, 64
```
input2:
```
    mstore 100, 0
    mstore 132, 0
```
output2:
```
    %1 = calldatasize calldatacopy 100, %1, 64
```

It handles copies from memory (mload/mstore/mcopy) and calldata
(calldataload/calldatacopy). For zeroing memory it uses `calldatacopy`
from the `calldatasize` offset.

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@charles-cooper
Copy link
Copy Markdown
Member

looking very nice

@HodanPlodky HodanPlodky changed the title feat[venom]: Memory write merging feat[venom]: memory write merging Nov 5, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 6, 2024

Codecov Report

Attention: Patch coverage is 98.75519% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.26%. Comparing base (ebe26a6) to head (b5b3566).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
vyper/venom/passes/memmerging.py 99.13% 2 Missing ⚠️
vyper/utils.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4341      +/-   ##
==========================================
- Coverage   91.12%   90.26%   -0.86%     
==========================================
  Files         115      116       +1     
  Lines       16235    16476     +241     
  Branches     2732     2778      +46     
==========================================
+ Hits        14794    14872      +78     
- Misses       1004     1141     +137     
- Partials      437      463      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

MemMergePass(ac, fn).run_pass()

bb = next(next(iter(ctx.functions.values())).get_basic_blocks())
assert bb.instructions[0].opcode == "mcopy"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think here we could have pre/post comparison

Copy link
Copy Markdown
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

lgtm. @HodanPlodky added some final tweaks to the test comments, will merge if they are ok

@charles-cooper charles-cooper changed the title feat[venom]: memory write merging feat[venom]: coalesce memory writes Dec 18, 2024
@charles-cooper charles-cooper changed the title feat[venom]: coalesce memory writes feat[venom]: merge memory writes Dec 18, 2024
@charles-cooper charles-cooper merged commit e20c363 into vyperlang:master Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants