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

Fix statics issue with barriers #108311

Merged
merged 2 commits into from
Sep 27, 2024
Merged

Fix statics issue with barriers #108311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7dab12e
Fix statics issue with barriers
davidwrighton Sep 27, 2024
a9cf91a
Add barriers for RiscV and Loongson
davidwrighton Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/coreclr/vm/arm64/asmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ DynamicHelper DynamicHelperFrameFlags_ObjectArg | DynamicHelperFrameFlags_Object

LEAF_ENTRY JIT_GetDynamicNonGCStaticBase_SingleAppDomain, _TEXT
// If class is not initialized, bail to C++ helper
ldr x1, [x0, #OFFSETOF__DynamicStaticsInfo__m_pNonGCStatics]
add x1, x0, #OFFSETOF__DynamicStaticsInfo__m_pNonGCStatics
ldar x1, [x1]
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the barriers in arm32 version of these helpers as well?

Copy link
Member

Choose a reason for hiding this comment

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

(And also riscv and loongarch.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we do, this was more intended to get a quick bit written up so that we could run some tests.

Copy link
Member

Choose a reason for hiding this comment

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

NOTE: can be ldapur x1, [x1, #OFFSETOF__DynamicStaticsInfo__m_pGCStatics] for macos-arm64 since that has rcpc2 in the baseline, but not sure it worth the complexity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, it's a detail that we will be able to do a better job of implementing these helpers when we move them to managed. (Since the jit will take advantage of the ldapr or ldapur instructions instead of just using ldar which is a stronger barrier than we need.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Finally, there is no need to fix arm, as we don't have handwritten assembly helpers for this set of helpers.

tbnz x1, #0, LOCAL_LABEL(JIT_GetDynamicNonGCStaticBase_SingleAppDomain_CallHelper)
mov x0, x1
ret lr
Expand All @@ -668,7 +669,8 @@ LEAF_END JIT_GetDynamicNonGCStaticBase_SingleAppDomain, _TEXT

LEAF_ENTRY JIT_GetDynamicGCStaticBase_SingleAppDomain, _TEXT
// If class is not initialized, bail to C++ helper
ldr x1, [x0, #OFFSETOF__DynamicStaticsInfo__m_pGCStatics]
add x1, x0, #OFFSETOF__DynamicStaticsInfo__m_pGCStatics
ldar x1, [x1]
tbnz x1, #0, LOCAL_LABEL(JIT_GetDynamicGCStaticBase_SingleAppDomain_CallHelper)
mov x0, x1
ret lr
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/vm/arm64/asmhelpers.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,8 @@ Fail

LEAF_ENTRY JIT_GetDynamicNonGCStaticBase_SingleAppDomain
; If class is not initialized, bail to C++ helper
ldr x1, [x0, #OFFSETOF__DynamicStaticsInfo__m_pNonGCStatics]
add x1, x0, #OFFSETOF__DynamicStaticsInfo__m_pNonGCStatics
ldar x1, [x1]
tbnz x1, #0, CallHelper1
mov x0, x1
ret lr
Expand All @@ -1026,7 +1027,8 @@ CallHelper1

LEAF_ENTRY JIT_GetDynamicGCStaticBase_SingleAppDomain
; If class is not initialized, bail to C++ helper
ldr x1, [x0, #OFFSETOF__DynamicStaticsInfo__m_pGCStatics]
add x1, x0, #OFFSETOF__DynamicStaticsInfo__m_pGCStatics
ldar x1, [x1]
tbnz x1, #0, CallHelper2
mov x0, x1
ret lr
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/loongarch64/asmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ LEAF_END setFPReturn, _TEXT
LEAF_ENTRY JIT_GetDynamicNonGCStaticBase_SingleAppDomain, _TEXT
// If class is not initialized, bail to C++ helper
ld.d $a1, $a0, OFFSETOF__DynamicStaticsInfo__m_pNonGCStatics
dbar 0
bnez $a1, LOCAL_LABEL(JIT_GetDynamicNonGCStaticBase_SingleAppDomain_CallHelper)
ori $a0, $a1, 0
jirl $r0, $ra, 0
Expand All @@ -952,6 +953,7 @@ LEAF_END JIT_GetDynamicNonGCStaticBase_SingleAppDomain, _TEXT
LEAF_ENTRY JIT_GetDynamicGCStaticBase_SingleAppDomain, _TEXT
// If class is not initialized, bail to C++ helper
ld.d $a1, $a0, OFFSETOF__DynamicStaticsInfo__m_pGCStatics
dbar 0
bnez $a1, LOCAL_LABEL(JIT_GetDynamicGCStaticBase_SingleAppDomain_CallHelper)
ori $a0, $a1, 0
jirl $r0, $ra, 0
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/vm/methodtable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,12 +603,12 @@ struct DynamicStaticsInfo
TADDR m_pGCStatics; // Always access through helper methods to properly handle the ISCLASSNOTINITED bit
TADDR m_pNonGCStatics; // Always access through helper methods to properly handle the ISCLASSNOTINITED bit
PTR_MethodTable m_pMethodTable;
PTR_OBJECTREF GetGCStaticsPointer() { TADDR staticsVal = VolatileLoadWithoutBarrier(&m_pGCStatics); return dac_cast<PTR_OBJECTREF>(staticsVal & STATICSPOINTERMASK); }
PTR_BYTE GetNonGCStaticsPointer() { TADDR staticsVal = VolatileLoadWithoutBarrier(&m_pNonGCStatics); return dac_cast<PTR_BYTE>(staticsVal & STATICSPOINTERMASK); }
PTR_OBJECTREF GetGCStaticsPointer() { TADDR staticsVal = VolatileLoad(&m_pGCStatics); return dac_cast<PTR_OBJECTREF>(staticsVal & STATICSPOINTERMASK); }
PTR_BYTE GetNonGCStaticsPointer() { TADDR staticsVal = VolatileLoad(&m_pNonGCStatics); return dac_cast<PTR_BYTE>(staticsVal & STATICSPOINTERMASK); }
PTR_OBJECTREF GetGCStaticsPointerAssumeIsInited() { TADDR staticsVal = m_pGCStatics; _ASSERTE(staticsVal != 0); _ASSERTE((staticsVal & (ISCLASSNOTINITEDMASK)) == 0); return dac_cast<PTR_OBJECTREF>(staticsVal); }
PTR_BYTE GetNonGCStaticsPointerAssumeIsInited() { TADDR staticsVal = m_pNonGCStatics; _ASSERTE(staticsVal != 0); _ASSERTE((staticsVal & (ISCLASSNOTINITEDMASK)) == 0); return dac_cast<PTR_BYTE>(staticsVal); }
bool GetIsInitedAndGCStaticsPointerIfInited(PTR_OBJECTREF *ptrResult) { TADDR staticsVal = VolatileLoadWithoutBarrier(&m_pGCStatics); *ptrResult = dac_cast<PTR_OBJECTREF>(staticsVal); return !(staticsVal & ISCLASSNOTINITED); }
bool GetIsInitedAndNonGCStaticsPointerIfInited(PTR_BYTE *ptrResult) { TADDR staticsVal = VolatileLoadWithoutBarrier(&m_pNonGCStatics); *ptrResult = dac_cast<PTR_BYTE>(staticsVal); return !(staticsVal & ISCLASSNOTINITED); }
bool GetIsInitedAndGCStaticsPointerIfInited(PTR_OBJECTREF *ptrResult) { TADDR staticsVal = VolatileLoad(&m_pGCStatics); *ptrResult = dac_cast<PTR_OBJECTREF>(staticsVal); return !(staticsVal & ISCLASSNOTINITED); }
bool GetIsInitedAndNonGCStaticsPointerIfInited(PTR_BYTE *ptrResult) { TADDR staticsVal = VolatileLoad(&m_pNonGCStatics); *ptrResult = dac_cast<PTR_BYTE>(staticsVal); return !(staticsVal & ISCLASSNOTINITED); }

// This function sets the pointer portion of a statics pointer. It returns false if the statics value was already set.
bool InterlockedUpdateStaticsPointer(bool isGC, TADDR newVal, bool isClassInitedByUpdatingStaticPointer)
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/riscv64/asmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,7 @@ LEAF_END setFPReturn, _TEXT
LEAF_ENTRY JIT_GetDynamicNonGCStaticBase_SingleAppDomain, _TEXT
// If class is not initialized, bail to C++ helper
ld a1, OFFSETOF__DynamicStaticsInfo__m_pNonGCStatics(a0)
fence r,rw
bnez a1, LOCAL_LABEL(JIT_GetDynamicNonGCStaticBase_SingleAppDomain_CallHelper)
mv a0, a1
ret
Expand All @@ -822,6 +823,7 @@ LEAF_END JIT_GetDynamicNonGCStaticBase_SingleAppDomain, _TEXT
LEAF_ENTRY JIT_GetDynamicGCStaticBase_SingleAppDomain, _TEXT
// If class is not initialized, bail to C++ helper
ld a1, OFFSETOF__DynamicStaticsInfo__m_pGCStatics(a0)
fence r,rw
bnez a1, LOCAL_LABEL(JIT_GetDynamicGCStaticBase_SingleAppDomain_CallHelper)
mv a0, a1
ret
Expand Down