Skip to content

Conversation

@jacobly0
Copy link
Member

No description provided.

@jacobly0 jacobly0 requested review from kubkon and mlugg August 26, 2024 20:39
Copy link
Member

@kubkon kubkon left a comment

Choose a reason for hiding this comment

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

Perhaps it would make sense to land elf-incr branch first (#21212) in its current state since it cleans up a lot of things in the ELF linker, and then rebase this branch on top? Next, initSyntheticSections really should not be used in initMetadata - happy to clean that one up if you want. Finally, not a blocker but as it currently stands, .eh_frame generated with ZigObject will not have its CIEs deduped across all input object files - not a biggie, but something to remember about. I would also like to have it tested with the linker - emit .eh_frame section as part of .o file and then link it back into an executable.

@jacobly0
Copy link
Member Author

Finally, not a blocker but as it currently stands, .eh_frame generated with ZigObject will not have its CIEs deduped across all input object files - not a biggie, but something to remember about. I would also like to have it tested with the linker - emit .eh_frame section as part of .o file and then link it back into an executable.

A CIE is like 24 bytes, so it's definitely not worth deduping the two extra CIE per module in incremental mode (the only use case I'm working towards right now), especially given all the padding that already exists. The key feature that would be useful is the ability to generate a .eh_frame_hdr in non-incremental mode, which I believe would come for free when CIE deduping is integrated.

Copy link
Member

@kubkon kubkon left a comment

Choose a reason for hiding this comment

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

Looks great!

@jacobly0 jacobly0 merged commit 1a178d4 into ziglang:master Aug 27, 2024
@jacobly0 jacobly0 deleted the eh-frame branch August 27, 2024 19:06
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.

3 participants