-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Unix arm64 atomics #71512
Unix arm64 atomics #71512
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
I tried to incorporate feedback of turning this always ON for OSX/Arm64. Since I don't have that machine, I tried to enable it for Unix/Arm64, just to see it compiles and behave correctly. Left= Ideal behaviour where we will use What I observe is, in Left, we just jump to the inlined implementation where as on right side, it still does some function multiversioning magic and goes to the (I believe) OS implementation of acq_release. See below the point where atomics instructions are executed. Here too, left is I will ask someone to validate what they see on OSX. |
@jkotas @janvorli @davidwrighton @dotnet/jit-contrib CC: @Maoni0 |
#if defined(LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT) | ||
|
||
#define Define_InterlockMethod(RETURN_TYPE, METHOD_DECL, METHOD_INVOC, INTRINSIC_NAME) \ | ||
EXTERN_C PALIMPORT inline RETURN_TYPE PALAPI METHOD_DECL \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the #71026 (comment), just to make sure that this does generate LSE, I believe this should also be attributed with "lse" and (noinline)
Improvement son Linux-arm64: dotnet/perf-autofiling-issues#6770 |
Use clang's target feature to mark methods as "lse" which would let us use
atomics
instruction on machines that supports that feature and falling back to the existing ldar/stlr.Improvements on benchmarks mentioned in #70921 (comment):
MAIN numbers
PR numbers