Skip to content

Conversation

@rodiazet
Copy link
Contributor

@rodiazet rodiazet commented Dec 10, 2024

  • Add subassembly link references to parent linker object.
  • Offset accordingly to subassembly position in parent object bytecode

Depends on #15628. Merged.
Depends on #15633. Merged.

@github-actions

This comment was marked as resolved.

@rodiazet rodiazet force-pushed the eof-fix-subs-refs branch 3 times, most recently from 0b85e3f to e358c78 Compare December 10, 2024 13:51
@cameel cameel added has dependencies The PR depends on other PRs that must be merged first EOF labels Dec 11, 2024
cameel
cameel previously approved these changes Dec 11, 2024
Comment on lines 1659 to +1669
for (auto i: referencedSubIds)
ret.bytecode += m_subs[i]->assemble().bytecode;
{
size_t const subAssemblyPostionInParentObject = ret.bytecode.size();
auto const& subAssemblyLinkerObject = m_subs[i]->assemble();
// Append subassembly bytecode to the parent assembly result bytecode
ret.bytecode += subAssemblyLinkerObject.bytecode;
// Add subassembly link references to parent linker object.
// Offset accordingly to subassembly position in parent object bytecode
for (auto const& [subAssemblyLinkRefPosition, linkRef]: subAssemblyLinkerObject.linkReferences)
ret.linkReferences[subAssemblyPostionInParentObject + subAssemblyLinkRefPosition] = linkRef;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that this still does not include the subassembly deduplication we introduced for legacy recently (#13804). I guess we can't do it the same way for EOF anyway since sub-assembly IDs are used as immediate arguments in some opcodes. We'd have to de-duplicate it at a higher level - in Yul or evmasm optimizer.

@rodiazet rodiazet dismissed cameel’s stale review December 11, 2024 20:25

The merge-base changed after approval.

@cameel cameel removed the has dependencies The PR depends on other PRs that must be merged first label Dec 12, 2024
cameel
cameel previously approved these changes Dec 12, 2024
Copy link
Collaborator

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Just needs a rebase.

@rodiazet rodiazet dismissed cameel’s stale review December 12, 2024 00:09

The merge-base changed after approval.

@cameel cameel merged commit 3b3586c into argotorg:develop Dec 12, 2024
73 checks passed
@rodiazet rodiazet deleted the eof-fix-subs-refs branch December 17, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants