Skip to content

Fix write barriers in NativeAOT - #74325

Merged
AntonLapounov merged 1 commit into
dotnet:mainfrom
AntonLapounov:FixWriteBarriers
Aug 22, 2022
Merged

Fix write barriers in NativeAOT#74325
AntonLapounov merged 1 commit into
dotnet:mainfrom
AntonLapounov:FixWriteBarriers

Conversation

@AntonLapounov

@AntonLapounovAntonLapounov commented Aug 22, 2022

Copy link
Copy Markdown
Contributor

For ARM64, use unsigned address comparisons in write barriers. In the INSERT_CHECKED_WRITE_BARRIER_CORE macro use the ccmp instruction to optimize for the case when the destination belongs to the heap.

For all architectures, make sure to exclude the g_GCShadowEnd, g_ephemeral_high, and g_highest_address upper bounds in the corresponding range checks. That is the current behavior on x64 except for g_GCShadowEnd.

Fixes#72645.

Comment threadsrc/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm
@VSadov

Copy link
Copy Markdown
Member

/azp run runtime-extra-platforms

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@VSadov

VSadov commented Aug 22, 2022

Copy link
Copy Markdown
Member

I am running a scenario that used to fail ~50% with this fix and there are no failures after 10 iterations. This issue was definitely responsible for most failures.

@mangod9

Copy link
Copy Markdown
Member

so looks like this fix is specific for NativeAOT, but vanilla crossgen2 arm64 failures need further investigation?

@VSadov

Copy link
Copy Markdown
Member

Sadly there were still quite a few failures when i left it run overnight. About 30 core dumps out of 108 rebuilds.
I think this fix helped, but there is something else.

@VSadovVSadov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!!

@VSadov

Copy link
Copy Markdown
Member

so looks like this fix is specific for NativeAOT, but vanilla crossgen2 arm64 failures need further investigation?

yes, this change would have no effect on CoreCLR

@AntonLapounov
AntonLapounov merged commit 0fe57ca into dotnet:mainAug 22, 2022
@AntonLapounov
AntonLapounov deleted the FixWriteBarriers branch August 22, 2022 19:06
@AntonLapounov

Copy link
Copy Markdown
ContributorAuthor

/backport to release/7.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0-rc1: https://github.com/dotnet/runtime/actions/runs/2906418073

@github-actions

Copy link
Copy Markdown
Contributor

@AntonLapounov backporting to release/7.0-rc1 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@AntonLapounov

Copy link
Copy Markdown
ContributorAuthor

About 30 core dumps out of 108 rebuilds.

Did you use Michal's repro or a different one?

@VSadov

Copy link
Copy Markdown
Member

Did you use #72645 (comment) or a different one?

no, I just build a subset or coreclr in a loop, which uses the currently built crossgen2 to build other stuff.

@AntonLapounov

Copy link
Copy Markdown
ContributorAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2914545828

@ghostghost locked as resolved and limited conversation to collaborators Sep 23, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linux ARM64 dotnet7 regression preview 4 to preview 5, 6 and 7

4 participants

@AntonLapounov@VSadov@mangod9@AaronRobinsonMSFT