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.

Sign up for GitHub

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

Jump to bottom

[ARM64] Add g_GCShadowEnd to JIT_WriteBarrier_Table #91342

Merged
merged 4 commits into from
Sep 1, 2023
Merged

[ARM64] Add g_GCShadowEnd to JIT_WriteBarrier_Table #91342

merged 4 commits into from
Sep 1, 2023

Conversation

alpencolt
Copy link

This change moves address of g_GCShadowEnd to JIT_WriteBarrier_Table like others variables used in Write Barrier.

This fix simmilar to RISC-V one #90036
cc @jakobbotsch @Maoni0 @wscho77 @HJLeee @JongHeonChoi @t-mustafin @clamp03 @gbalykov @tomeksowi

[ARM64] Add g_GCShadowEnd to JIT_WriteBarrier_Table
bdbd5b3 
This change moves address of g_GCShadowEnd to JIT_WriteBarrier_Table like
others variables used in Write Barrier.

This fix simmilar to RISC-V one dotnet#90036
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 30, 2023
jkotas
jkotas reviewed Aug 30, 2023
View reviewed changes
src/coreclr/vm/arm64/asmhelpers.asm Outdated
Comment on lines 350 to 353
wbs_GCShadow
DCQ 0
wbs_GCShadowEnd
DCQ 0
Copy link
Member

Choose a reason for hiding this comment

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

Could you please move wbs_GCShadow and wbs_GCShadowEnd to the end of the wbs block? These are debug-only fields, having them at the end would be a tiny bit better for cache locality.

Copy link
Author

Choose a reason for hiding this comment

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

@jkotas moved to the end of the wbs

jkotas
jkotas approved these changes Aug 31, 2023
View reviewed changes
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

Maoni0
Maoni0 approved these changes Aug 31, 2023
View reviewed changes
Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this work!

This was referenced Aug 31, 2023
Closed
Closed
@jkotas
Copy link
Member

jkotas commented Sep 1, 2023

Build errors:

D:\a\_work\1\s\artifacts\obj\coreclr\windows.arm64.Checked\vm\wks\asmhelpers.asm(5194) : error A2003: improper line syntax: illegal operand syntax

        adrp     x12, g_GCShadow
D:\a\_work\1\s\artifacts\obj\coreclr\windows.arm64.Checked\vm\wks\asmhelpers.asm(5197) : error A2003: improper line syntax: illegal operand syntax

@jkotas jkotas merged commit d8b177e into dotnet:main Sep 1, 2023
109 checks passed
@EgorBo EgorBo mentioned this pull request Sep 7, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkotas jkotas jkotas approved these changes

@Maoni0 Maoni0 Maoni0 approved these changes

Assignees

@alpencolt alpencolt

Labels
area-VM-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alpencolt @jkotas @Maoni0