Fix statics issue with barriers #108311
Fix statics issue with barriers #108311
Conversation
| @@ -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] | |||
There was a problem hiding this comment.
Do we need the barriers in arm32 version of these helpers as well?
There was a problem hiding this comment.
Yes we do, this was more intended to get a quick bit written up so that we could run some tests.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
Finally, there is no need to fix arm, as we don't have handwritten assembly helpers for this set of helpers.
|
Tagging subscribers to this area: @mangod9 |
|
/backport to release/9.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11076638514 |
|
@davidwrighton backporting to release/9.0 failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix statics issue with barriers
Applying: Add barriers for RiscV and Loongson
Using index info to reconstruct a base tree...
M src/coreclr/vm/loongarch64/asmhelpers.S
M src/coreclr/vm/riscv64/asmhelpers.S
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/vm/riscv64/asmhelpers.S
CONFLICT (content): Merge conflict in src/coreclr/vm/riscv64/asmhelpers.S
Auto-merging src/coreclr/vm/loongarch64/asmhelpers.S
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0002 Add barriers for RiscV and Loongson
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@davidwrighton an error occurred while backporting to release/9.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
* Fix statics issue with barriers * Add barriers for RiscV and Loongson
* Fix statics issue with barriers * Add barriers for RiscV and Loongson
A barrier based approach for fixing issue #105441
This adds barriers after all the places outside of jitted code where we check to see if the class constructor is initialized. These barriers are somewhat non-optimal on ARM64, as they use ldar instead of ldapr, but should fix the problems we've been encountering with regards to memory model behavior around statics.