-
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
RyuJIT: Remove redundant memory barrier for XAdd and XChg on arm #45970
Conversation
GetEmitter()->emitIns_R_R_R(INS_ldaddal, dataSize, dataReg, targetReg, addrReg); | ||
} | ||
GetEmitter()->emitIns_R_R_R(INS_ldaddal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, | ||
addrReg); |
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.
NOTE: if targetReg is REG_NA
(means we don't care about Interlocked.Add
's return value) it uses WZR
as a target register (it ignores writes and is always zero)
The trailing With |
LGTM |
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.
LGTM
As far as I understand that memory barrier is not needed when we already emit LDADDAL and SWPAL with "acquire and release" semantics.
UPD: same for CASAL (emitted for
Interlocked.CompareExchange
)For this reason I also replaced
staddl
withldaddal
for the case when we don't need the return value ofInterlocked.Add
.Just like LLVM does: https://godbolt.org/z/a9GcT8
Here are the diff examples:
G_M16300_IG01: A9BF7BFD stp fp, lr, [sp,#-16]! 910003FD mov fp, sp G_M16300_IG02: B8E10000 ldaddal w1, w0, [x0] - D5033BBF dmb ish 0B010000 add w0, w0, w1 G_M16300_IG03: A8C17BFD ldp fp, lr, [sp],#16 D65F03C0 ret lr
G_M24897_IG01: A9BF7BFD stp fp, lr, [sp,#-16]! 910003FD mov fp, sp G_M24897_IG02: B8E18000 swpal w1, w0, [x0] - D5033BBF dmb ish G_M24897_IG03: A8C17BFD ldp fp, lr, [sp],#16 D65F03C0 ret lr
/cc @dotnet/jit-contrib