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

ArmEmitter: Support single use forward labels #3363

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

Sonicadvance1
Copy link
Member

@Sonicadvance1 Sonicadvance1 commented Jan 9, 2024

Needs #3361 merged first.

Previously forward labels needed to be pessimistic about multiple instructions binding to a label. This results in forward labels having a vector that always allocates memory. Instead if we know the data is only going to be used by a single instruction then we don't need to allocate any memory. This is the common case in FEX.

In particular the delinker step for the JIT blocks was always allocating memory due to the forward label. Now it no longer allocates any memory since it uses a single use forward label.

The delinker step of the JIT was using std::function with capture
lambdas that required memory allocation when unnecessary.
Because the compiler can't see through our std::function usage it could
never decompose these by itself.

By passing the Thread's frame and record to the function as arguments
then we can have the signature be a raw function pointer.

This fixes an area of concern from:
https://github.com/FEX-Emu/FEX/blob/main/docs/ProgrammingConcerns.md#stdfunction-and-lambdas
Currently all uses of the forward label calls in to jemalloc to allocate
memory. This allows a forward label that doesn't require any memory
allocation, which is the common case in FEX.
Primary goal for this is to ensure that the delinker doesn't need to
allocate any memory. This delinker can end up getting hit heavily with
JIT code so we don't want it to be allocating memory.
@Sonicadvance1 Sonicadvance1 force-pushed the fix_label_allocations branch from cc17bf6 to 3710240 Compare January 9, 2024 06:18
Copy link
Collaborator

@alyssarosenzweig alyssarosenzweig left a comment

Choose a reason for hiding this comment

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

nice!

@Sonicadvance1 Sonicadvance1 merged commit 8c31630 into FEX-Emu:main Jan 12, 2024
10 checks passed
@Sonicadvance1 Sonicadvance1 deleted the fix_label_allocations branch January 12, 2024 08:26
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.

2 participants