Skip to content

Commit 07f7626

Browse files
EgorBojkotas
andauthored
ARM64: avoid memory barrier in InlinedMemmoveGCRefsHelper if dest is not on heap (#102084)
* don't put full barrier if dest is not on the heap * Address feedback --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 7ce5d0f commit 07f7626

File tree

7 files changed

+36
-59
lines changed

7 files changed

+36
-59
lines changed

src/coreclr/classlibnative/bcltype/arraynative.inl

+10-2
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,12 @@ FORCEINLINE void InlinedMemmoveGCRefsHelper(void *dest, const void *src, size_t
307307
_ASSERTE(CheckPointer(dest));
308308
_ASSERTE(CheckPointer(src));
309309

310-
GCHeapMemoryBarrier();
310+
const bool notInHeap = ((BYTE*)dest < g_lowest_address || (BYTE*)dest >= g_highest_address);
311+
312+
if (!notInHeap)
313+
{
314+
GCHeapMemoryBarrier();
315+
}
311316

312317
// To be able to copy forwards, the destination buffer cannot start inside the source buffer
313318
if ((size_t)dest - (size_t)src >= len)
@@ -319,7 +324,10 @@ FORCEINLINE void InlinedMemmoveGCRefsHelper(void *dest, const void *src, size_t
319324
InlinedBackwardGCSafeCopyHelper(dest, src, len);
320325
}
321326

322-
InlinedSetCardsAfterBulkCopyHelper((Object**)dest, len);
327+
if (!notInHeap)
328+
{
329+
InlinedSetCardsAfterBulkCopyHelper((Object**)dest, len);
330+
}
323331
}
324332

325333
#endif // !_ARRAYNATIVE_INL_

src/coreclr/nativeaot/Runtime/GCMemoryHelpers.cpp

+17-6
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,28 @@ FCIMPLEND
4444

4545
FCIMPL3(void, RhBulkMoveWithWriteBarrier, uint8_t* pDest, uint8_t* pSrc, size_t cbDest)
4646
{
47-
// It is possible that the bulk write is publishing object references accessible so far only
48-
// by the current thread to shared memory.
49-
// The memory model requires that writes performed by current thread are observable no later
50-
// than the writes that will actually publish the references.
51-
GCHeapMemoryBarrier();
47+
if (cbDest == 0 || pDest == pSrc)
48+
return;
49+
50+
const bool notInHeap = pDest < g_lowest_address || pDest >= g_highest_address;
51+
52+
if (!notInHeap)
53+
{
54+
// It is possible that the bulk write is publishing object references accessible so far only
55+
// by the current thread to shared memory.
56+
// The memory model requires that writes performed by current thread are observable no later
57+
// than the writes that will actually publish the references.
58+
GCHeapMemoryBarrier();
59+
}
5260

5361
if (pDest <= pSrc || pSrc + cbDest <= pDest)
5462
InlineForwardGCSafeCopy(pDest, pSrc, cbDest);
5563
else
5664
InlineBackwardGCSafeCopy(pDest, pSrc, cbDest);
5765

58-
InlinedBulkWriteBarrier(pDest, cbDest);
66+
if (!notInHeap)
67+
{
68+
InlinedBulkWriteBarrier(pDest, cbDest);
69+
}
5970
}
6071
FCIMPLEND

src/coreclr/nativeaot/Runtime/GCMemoryHelpers.inl

+3-8
Original file line numberDiff line numberDiff line change
@@ -228,14 +228,9 @@ FORCEINLINE void InlineCheckedWriteBarrier(void * dst, void * ref)
228228

229229
FORCEINLINE void InlinedBulkWriteBarrier(void* pMemStart, size_t cbMemSize)
230230
{
231-
// Check whether the writes were even into the heap. If not there's no card update required.
232-
// Also if the size is smaller than a pointer, no write barrier is required.
233-
// This case can occur with universal shared generic code where the size
234-
// is not known at compile time.
235-
if (pMemStart < g_lowest_address || (pMemStart >= g_highest_address) || (cbMemSize < sizeof(uintptr_t)))
236-
{
237-
return;
238-
}
231+
// Caller is expected to check whether the writes were even into the heap
232+
ASSERT(cbMemSize >= sizeof(uintptr_t));
233+
ASSERT((pMemStart >= g_lowest_address) && (pMemStart < g_highest_address));
239234

240235
#ifdef WRITE_BARRIER_CHECK
241236
// Perform shadow heap updates corresponding to the gc heap updates that immediately preceded this helper

src/coreclr/vm/gchelpers.cpp

+1-36
Original file line numberDiff line numberDiff line change
@@ -1484,39 +1484,4 @@ void ErectWriteBarrierForMT(MethodTable **dst, MethodTable *ref)
14841484
}
14851485
}
14861486
}
1487-
}
1488-
1489-
//----------------------------------------------------------------------------
1490-
//
1491-
// Write Barrier Support for bulk copy ("Clone") operations
1492-
//
1493-
// StartPoint is the target bulk copy start point
1494-
// len is the length of the bulk copy (in bytes)
1495-
//
1496-
//
1497-
// Performance Note:
1498-
//
1499-
// This is implemented somewhat "conservatively", that is we
1500-
// assume that all the contents of the bulk copy are object
1501-
// references. If they are not, and the value lies in the
1502-
// ephemeral range, we will set false positives in the card table.
1503-
//
1504-
// We could use the pointer maps and do this more accurately if necessary
1505-
1506-
#if defined(_MSC_VER) && defined(TARGET_X86)
1507-
#pragma optimize("y", on) // Small critical routines, don't put in EBP frame
1508-
#endif //_MSC_VER && TARGET_X86
1509-
1510-
void
1511-
SetCardsAfterBulkCopy(Object **start, size_t len)
1512-
{
1513-
// If the size is smaller than a pointer, no write barrier is required.
1514-
if (len >= sizeof(uintptr_t))
1515-
{
1516-
InlinedSetCardsAfterBulkCopyHelper(start, len);
1517-
}
1518-
}
1519-
1520-
#if defined(_MSC_VER) && defined(TARGET_X86)
1521-
#pragma optimize("", on) // Go back to command line default optimizations
1522-
#endif //_MSC_VER && TARGET_X86
1487+
}

src/coreclr/vm/gchelpers.h

-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ extern void ThrowOutOfMemoryDimensionsExceeded();
8787
//========================================================================
8888

8989
void ErectWriteBarrier(OBJECTREF* dst, OBJECTREF ref);
90-
void SetCardsAfterBulkCopy(Object **start, size_t len);
9190

9291
void PublishFrozenObject(Object*& orObject);
9392

src/coreclr/vm/gchelpers.inl

+2-6
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,9 @@
3131

3232
FORCEINLINE void InlinedSetCardsAfterBulkCopyHelper(Object **start, size_t len)
3333
{
34-
// Check whether the writes were even into the heap. If not there's no card update required.
35-
// Also if the size is smaller than a pointer, no write barrier is required.
34+
// Caller is expected to check whether the writes were even into the heap
3635
_ASSERTE(len >= sizeof(uintptr_t));
37-
if ((BYTE*)start < g_lowest_address || (BYTE*)start >= g_highest_address)
38-
{
39-
return;
40-
}
36+
_ASSERTE(((BYTE*)start >= g_lowest_address) && ((BYTE*)start < g_highest_address));
4137

4238
// Don't optimize the Generation 0 case if we are checking for write barrier violations
4339
// since we need to update the shadow heap even in the generation 0 case.

src/mono/mono/metadata/icall.c

+3
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,9 @@ ves_icall_System_Runtime_RuntimeImports_Memmove (guint8 *destination, guint8 *so
957957
void
958958
ves_icall_System_Buffer_BulkMoveWithWriteBarrier (guint8 *destination, guint8 *source, size_t len, MonoType *type)
959959
{
960+
if (len == 0 || destination == source)
961+
return;
962+
960963
if (MONO_TYPE_IS_REFERENCE (type))
961964
mono_gc_wbarrier_arrayref_copy_internal (destination, source, (guint)len);
962965
else

0 commit comments

Comments
 (0)