Skip to content

Modularize IR generator output#15229

Closed
cameel wants to merge 3 commits intodevelopfrom
modular-ir-generator-output
Closed

Modularize IR generator output#15229
cameel wants to merge 3 commits intodevelopfrom
modular-ir-generator-output

Conversation

@cameel
Copy link
Collaborator

@cameel cameel commented Jul 1, 2024

The first part of refactor for #15179.
Replaces #15228.

The PR introduces IRGeneratorOutput - a container that represents Yul object structure without having to serialize it all into a string. Dependencies are represented by pointers to contracts and can be inserted at the time when the object is printed.

I changed the approach from #15228 by putting the new container in the codegen rather than trying to fit it into Yul ObjectNode hierarchy. The original idea still has some merit, but it seemed slightly off in all kinds of small ways that I couldn't properly resolve. libyul just does not seem like the right layer for it. It being strictly tied to the codegen now also lets me make more assumptions about the structure and simplify the processing.

@ekpyron
Copy link
Collaborator

ekpyron commented Jul 2, 2024

Funnily, I just had a brief first look at this and #15230 and thought this was way more complicated than necessary and was about to suggest simpler approaches - but while trying to write a suggestion realized that, unfortunately, it probably actually doesn't get simpler than this :-). I thought about just caching the optimized Yul code for each ContractDefinition, but that probably won't work without polluting the yul objects by relating them back to the contract they come from, which is horribly ugly - which leaves generating them uncombined and only combining them from the outside...

@ekpyron
Copy link
Collaborator

ekpyron commented Jul 2, 2024

Still, I'm wondering... this will only work for one-step compilation and not if we start from Yul sources... for that we'd need to do something like keccak hashing the code blocks of the Yul objects and store the optimized Yul for them per compiler invocation or something like that... which could also actually be simpler... but I haven't thought this through entirely yet, I'm just wondering.

@cameel
Copy link
Collaborator Author

cameel commented Jul 16, 2024

@ekpyron Do you think there's still some value in introducing this IRGeneratorOutput struct or should I just close this?

@ekpyron
Copy link
Collaborator

ekpyron commented Jul 16, 2024

Well, it does complicate things and doesn't bring that much benefit - so I'd close it for now.

@cameel cameel closed this Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants