Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NativeAOT] ARM: Implement the EABI #97604

Merged
merged 8 commits into from
Jan 30, 2024
Merged

Conversation

filipnavara
Copy link
Member

@filipnavara filipnavara commented Jan 27, 2024

This replaces DWARF exception unwinding on linux-arm with the compact EHABI representation. It also emits the .ARM.attributes section to aid debuggers and linkers.

Majority of methods use single unwind word (4 bytes) to describe the method and two words in the index table (8 bytes). For something like the Exceptions smoke test we see the following file sizes of the Exceptions.o object file.

Before: 16,769,298 bytes
After: 16,431,639 bytes

So, we saved 337,659 bytes and that doesn't even include the EH frame index produced by the linker for DWARF. It's not entirely fair comparison since the DWARF information emitted on ARM was overly verbose but it's a real size saving.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jan 27, 2024
@ghost
Copy link

ghost commented Jan 27, 2024

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

This replaces DWARF exception unwinding on linux-arm with the compact EHABI representation. It also emits the .ARM.attributes section to aid debuggers and linkers.

Kept as draft since some code paths are missing. It passes the smoke tests though.

Author: filipnavara
Assignees: -
Labels:

community-contribution, area-NativeAOT-coreclr

Milestone: -

src/mono/wasm/emsdk Outdated Show resolved Hide resolved

// The maximum sequence length of the ARM EHABI unwinding code is 1024
// bytes.
byte[] unwindData = new byte[1024];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
byte[] unwindData = new byte[1024];
var unwindData = new ArrayBuilder<byte>();

I assume that the blob is typically much smaller. Allocating full 1024 bytes every time looks wasteful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. It was originally a stackalloc but I could not use that with the local methods. I rewrote it to use ArrayPool now, to match the existing code in DwarfFde. That said, it's probably something that can be rewritten not to allocate at all. I will likely revisit it at later time.

SectionWriter extabSectionWriter;

if (ShouldShareSymbol((ObjectNode)nodeWithCodeInfo) ||
sectionWriter.SectionIndex != _managedCodeSectionIndex)
Copy link
Member

Choose a reason for hiding this comment

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

What is this special casing of _managedCodeSectionIndex about?

Copy link
Member Author

Choose a reason for hiding this comment

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

The specification allows either one .ARM.exidx/.ARM.extab section per code section, or one for each method. This is enforced to some extent by the format since the .ARM.exidx link field has to point to the corresponding code section.

Since we currently produce the unwinding info only for the managed code section this was merely a shortcut to use a single unwinding section for managed code and fallback to one per function for everything else (realistically only COMDAT handled by the first condition at this point).

Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively I can update the code to properly produce .ARM.exidx/.ARM.extab for each section and get rid of the condition...

Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively I can update the code to properly produce .ARM.exidx/.ARM.extab for each section and get rid of the condition...

Done

@jkotas
Copy link
Member

jkotas commented Jan 29, 2024

/azp run runtime-nativeaot-outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas jkotas merged commit 5e80e3e into dotnet:main Jan 30, 2024
191 of 196 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants