Codegen #676
Codegen #676
Conversation
Codecov Report
@@ Coverage Diff @@
## master #676 +/- ##
==========================================
- Coverage 81.95% 81.67% -0.28%
==========================================
Files 125 105 -20
Lines 10905 11821 +916
==========================================
+ Hits 8937 9655 +718
- Misses 1968 2166 +198
Continue to review full report at Codecov.
|
Y-Nak
force-pushed
the
codegen
branch
3 times, most recently
from
c2a2cd3
to
f40c5d1
Compare
Y-Nak
force-pushed
the
codegen
branch
7 times, most recently
from
609092c
to
aaf633f
Compare
Y-Nak
force-pushed
the
codegen
branch
12 times, most recently
from
aaf9eaa
to
2c40774
Compare
There was a problem hiding this comment.
Epic work! I took about two hours to go over this and the code looks very well written and clean. 👍 🥷
But to be honest I would probably need days to digest it better. 🤯 So this is more like a rubber stamp approval. Chances are that @sbillig or @g-r-a-n-t will be able to provide better feedback.
crates/fe/src/main.rs
Outdated
| } | ||
| } | ||
| } else { | ||
| eprintln!("mir doesn't support ingot yet"); |
There was a problem hiding this comment.
It's not supported as far as the print out of yul is concerned or in general? Because the ingot tests seem to be working so I'm a bit confused what exactly it is about ingots that mir doesn't support yet.
There was a problem hiding this comment.
If the input is an ingot, then the printing out of MIR is not supported yet. All functionality except for the pretty-printing works fine. I made the message clearer.
| get_my_sto_tuple([]) used 922 gas | ||
| emit_my_event([Tuple([Uint(26), Bool(true), Address(0x0000000000000000000000000000000000000042)])]) used 2484 gas | ||
| build_tuple_and_emit([]) used 2537 gas | ||
| encode_my_tuple([Tuple([Uint(26), Bool(true), Address(0x0000000000000000000000000000000000000042)])]) used 1205 gas |
There was a problem hiding this comment.
Looks like we are getting some nice gas savings out of this ❤️
There was a problem hiding this comment.
🕺 looks great. Sorry it took me a while. I'd say merge whenever you're ready.
I noticed a couple minor things that could be cleaned up now that the old lowering pass is gone, namely the Ingot item's original field, ModuleSource::Lowered (and maybe more?), but someone can do a small cleanup pr later of course.
|
@sbillig I opened an issue for cleaning up the leftovers. |
close #249, close #442, close #590, close #671, close #681, close #709
Overview
fe-codegenwhich lowermirintoyul, also it will be used to lowermirintosonatina-ir.fe-loweringandfe-yulgenStorage layout
The new storage layout is almost the same as the solidity's one, which is described in here
Memory layout
Unlike solidity, all primitive and aggregate types are stored in the same manner with the storage layout.
Also, the initial 96 bytes in memory are reserved as solidity does.
Event type
eventtypes are treated in the same way as "normal" struct types to remove ad-hoc implementation specialized for event types. By regarding event types as normal struct types, we could easily allow event types to contain aggregate types for example.But this abstraction makes gas efficiency worse because we should allocate memory every time when events appear.
e.g.
In this case 64 bytes are allocated for
MyEvent, while oldyulgendoesn't allocate it.Followup
#716
OPTIONAL: Update Spec if applicable
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history