Skip to content

Commit 3f4e17a

Browse files
committed
Use LoadBarrier on the managed side as well.
1 parent a5d5932 commit 3f4e17a

File tree

15 files changed

+72
-26
lines changed

15 files changed

+72
-26
lines changed

src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,12 @@ private static CastResult TryGet(nuint source, nuint target)
131131
if (entryTargetAndResult <= 1)
132132
{
133133
// make sure 'version' is loaded after 'source' and 'targetAndResults'
134-
#if TARGET_ARM64 || TARGET_ARM
134+
//
135135
// We can either:
136136
// - use acquires for both _source and _targetAndResults or
137-
// - issue a full fence before reading _version
138-
// benchmarks on available hardware show that use of full fence here is cheaper.
139-
Interlocked.MemoryBarrier();
140-
#endif
137+
// - issue a load barrier before reading _version
138+
// benchmarks on available hardware show that use of a load barrier is cheaper.
139+
Interlocked.LoadBarrier();
141140
if (version != pEntry._version)
142141
{
143142
// oh, so close, the entry is in inconsistent state.

src/coreclr/src/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,15 @@ public static long Read(ref long location) =>
232232
[MethodImpl(MethodImplOptions.InternalCall)]
233233
public static extern void MemoryBarrier();
234234

235+
/// <summary>
236+
/// Synchronizes memory access as follows:
237+
/// The processor that executes the current thread cannot reorder instructions in such a way that memory reads before
238+
/// the call to <see cref="LoadBarrier"/> execute after memory accesses that follow the call to <see cref="LoadBarrier"/>.
239+
/// </summary>
240+
[Intrinsic]
241+
[MethodImpl(MethodImplOptions.InternalCall)]
242+
internal static extern void LoadBarrier();
243+
235244
[DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
236245
private static extern void _MemoryBarrierProcessWide();
237246

src/coreclr/src/inc/corinfo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,7 @@ enum CorInfoIntrinsics
947947
CORINFO_INTRINSIC_InterlockedCmpXchg32,
948948
CORINFO_INTRINSIC_InterlockedCmpXchg64,
949949
CORINFO_INTRINSIC_MemoryBarrier,
950+
CORINFO_INTRINSIC_MemoryBarrierLoad,
950951
CORINFO_INTRINSIC_GetCurrentManagedThread,
951952
CORINFO_INTRINSIC_GetManagedThreadId,
952953
CORINFO_INTRINSIC_ByReference_Ctor,

src/coreclr/src/jit/codegen.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,11 +1465,13 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
14651465

14661466
void instGen_Return(unsigned stkArgSize);
14671467

1468-
#ifdef TARGET_ARM64
1469-
void instGen_MemoryBarrier(insBarrier barrierType = INS_BARRIER_ISH);
1470-
#else
1471-
void instGen_MemoryBarrier();
1472-
#endif
1468+
enum BarrierKind
1469+
{
1470+
BARRIER_FULL, // full barrier
1471+
BARRIER_LOAD_ONLY, // load barier
1472+
};
1473+
1474+
void instGen_MemoryBarrier(BarrierKind barrierKind = BARRIER_FULL);
14731475

14741476
void instGen_Set_Reg_To_Zero(emitAttr size, regNumber reg, insFlags flags = INS_FLAGS_DONT_CARE);
14751477

src/coreclr/src/jit/codegenarm64.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2666,7 +2666,7 @@ void CodeGen::genCodeForCpObj(GenTreeObj* cpObjNode)
26662666
if (cpObjNode->gtFlags & GTF_BLK_VOLATILE)
26672667
{
26682668
// issue a INS_BARRIER_ISHLD after a volatile CpObj operation
2669-
instGen_MemoryBarrier(INS_BARRIER_ISHLD);
2669+
instGen_MemoryBarrier(BARRIER_LOAD_ONLY);
26702670
}
26712671

26722672
// Clear the gcInfo for REG_WRITE_BARRIER_SRC_BYREF and REG_WRITE_BARRIER_DST_BYREF.
@@ -2775,7 +2775,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode)
27752775
assert(!"Unexpected treeNode->gtOper");
27762776
}
27772777

2778-
instGen_MemoryBarrier(INS_BARRIER_ISH);
2778+
instGen_MemoryBarrier();
27792779
}
27802780
else
27812781
{
@@ -2855,7 +2855,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode)
28552855

28562856
GetEmitter()->emitIns_J_R(INS_cbnz, EA_4BYTE, labelRetry, exResultReg);
28572857

2858-
instGen_MemoryBarrier(INS_BARRIER_ISH);
2858+
instGen_MemoryBarrier();
28592859

28602860
gcInfo.gcMarkRegSetNpt(addr->gtGetRegMask());
28612861
}
@@ -2904,7 +2904,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode)
29042904
}
29052905
GetEmitter()->emitIns_R_R_R(INS_casal, dataSize, targetReg, dataReg, addrReg);
29062906

2907-
instGen_MemoryBarrier(INS_BARRIER_ISH);
2907+
instGen_MemoryBarrier();
29082908
}
29092909
else
29102910
{
@@ -2984,7 +2984,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode)
29842984

29852985
genDefineTempLabel(labelCompareFail);
29862986

2987-
instGen_MemoryBarrier(INS_BARRIER_ISH);
2987+
instGen_MemoryBarrier();
29882988

29892989
gcInfo.gcMarkRegSetNpt(addr->gtGetRegMask());
29902990
}

src/coreclr/src/jit/codegenarmarch.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,13 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
398398
break;
399399

400400
case GT_MEMORYBARRIER:
401-
instGen_MemoryBarrier();
401+
{
402+
CodeGen::BarrierKind barrierKind =
403+
treeNode->gtFlags & GTF_MEMORYBARRIER_LOAD ? BARRIER_LOAD_ONLY : BARRIER_FULL;
404+
405+
instGen_MemoryBarrier(barrierKind);
402406
break;
407+
}
403408

404409
#ifdef TARGET_ARM64
405410
case GT_XCHG:
@@ -1945,7 +1950,7 @@ void CodeGen::genCodeForIndir(GenTreeIndir* tree)
19451950
if (emitBarrier)
19461951
{
19471952
#ifdef TARGET_ARM64
1948-
instGen_MemoryBarrier(INS_BARRIER_ISHLD);
1953+
instGen_MemoryBarrier(BARRIER_LOAD_ONLY);
19491954
#else
19501955
instGen_MemoryBarrier();
19511956
#endif
@@ -1982,7 +1987,7 @@ void CodeGen::genCodeForCpBlkHelper(GenTreeBlk* cpBlkNode)
19821987
{
19831988
#ifdef TARGET_ARM64
19841989
// issue a INS_BARRIER_ISHLD after a volatile CpBlk operation
1985-
instGen_MemoryBarrier(INS_BARRIER_ISHLD);
1990+
instGen_MemoryBarrier(BARRIER_LOAD_ONLY);
19861991
#else
19871992
// issue a full memory barrier after a volatile CpBlk operation
19881993
instGen_MemoryBarrier();
@@ -2305,7 +2310,7 @@ void CodeGen::genCodeForCpBlkUnroll(GenTreeBlk* node)
23052310
if (node->IsVolatile())
23062311
{
23072312
#ifdef TARGET_ARM64
2308-
instGen_MemoryBarrier(INS_BARRIER_ISHLD);
2313+
instGen_MemoryBarrier(BARRIER_LOAD_ONLY);
23092314
#else
23102315
instGen_MemoryBarrier();
23112316
#endif

src/coreclr/src/jit/codegenxarch.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1862,8 +1862,13 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
18621862
break;
18631863

18641864
case GT_MEMORYBARRIER:
1865-
instGen_MemoryBarrier();
1865+
{
1866+
CodeGen::BarrierKind barrierKind =
1867+
treeNode->gtFlags & GTF_MEMORYBARRIER_LOAD ? BARRIER_LOAD_ONLY : BARRIER_FULL;
1868+
1869+
instGen_MemoryBarrier(barrierKind);
18661870
break;
1871+
}
18671872

18681873
case GT_CMPXCHG:
18691874
genCodeForCmpXchg(treeNode->AsCmpXchg());

src/coreclr/src/jit/gentree.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,8 @@ struct GenTree
814814
#define GTF_CALL_POP_ARGS 0x04000000 // GT_CALL -- caller pop arguments?
815815
#define GTF_CALL_HOISTABLE 0x02000000 // GT_CALL -- call is hoistable
816816

817+
#define GTF_MEMORYBARRIER_LOAD 0x40000000 // GT_MEMORYBARRIER -- Load barrier
818+
817819
#define GTF_NOP_DEATH 0x40000000 // GT_NOP -- operand dies here
818820

819821
#define GTF_FLD_VOLATILE 0x40000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE

src/coreclr/src/jit/importer.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3627,11 +3627,20 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
36273627
#endif // defined(TARGET_XARCH) || defined(TARGET_ARM64)
36283628

36293629
case CORINFO_INTRINSIC_MemoryBarrier:
3630+
case CORINFO_INTRINSIC_MemoryBarrierLoad:
36303631

36313632
assert(sig->numArgs == 0);
36323633

36333634
op1 = new (this, GT_MEMORYBARRIER) GenTree(GT_MEMORYBARRIER, TYP_VOID);
36343635
op1->gtFlags |= GTF_GLOB_REF | GTF_ASG;
3636+
3637+
// on XARCH only full fences are emitted.
3638+
// there is still effect on reordering.
3639+
if (intrinsicID == CORINFO_INTRINSIC_MemoryBarrierLoad)
3640+
{
3641+
op1->gtFlags |= GTF_MEMORYBARRIER_LOAD;
3642+
}
3643+
36353644
retNode = op1;
36363645
break;
36373646

src/coreclr/src/jit/instr.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2364,11 +2364,7 @@ void CodeGen::instGen_Return(unsigned stkArgSize)
23642364
* Note: all MemoryBarriers instructions can be removed by
23652365
* SET COMPlus_JitNoMemoryBarriers=1
23662366
*/
2367-
#ifdef TARGET_ARM64
2368-
void CodeGen::instGen_MemoryBarrier(insBarrier barrierType)
2369-
#else
2370-
void CodeGen::instGen_MemoryBarrier()
2371-
#endif
2367+
void CodeGen::instGen_MemoryBarrier(BarrierKind barrierKind)
23722368
{
23732369
#ifdef DEBUG
23742370
if (JitConfig.JitNoMemoryBarriers() == 1)
@@ -2378,12 +2374,17 @@ void CodeGen::instGen_MemoryBarrier()
23782374
#endif // DEBUG
23792375

23802376
#if defined(TARGET_XARCH)
2377+
if (barrierKind != BARRIER_FULL)
2378+
{
2379+
return;
2380+
}
2381+
23812382
instGen(INS_lock);
23822383
GetEmitter()->emitIns_I_AR(INS_or, EA_4BYTE, 0, REG_SPBASE, 0);
23832384
#elif defined(TARGET_ARM)
23842385
GetEmitter()->emitIns_I(INS_dmb, EA_4BYTE, 0xf);
23852386
#elif defined(TARGET_ARM64)
2386-
GetEmitter()->emitIns_BARR(INS_dmb, barrierType);
2387+
GetEmitter()->emitIns_BARR(INS_dmb, barrierKind == BARRIER_LOAD_ONLY ? INS_BARRIER_ISHLD : INS_BARRIER_ISH);
23872388
#else
23882389
#error "Unknown TARGET"
23892390
#endif

0 commit comments

Comments
 (0)