From 60d00c47a69a6054ff82bec717cfc5a2d6ff23c6 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 5 Dec 2022 16:46:03 +0100 Subject: [PATCH] Intrinsify Interlocked.CompareExchange and Interlocked.Exchange for null (#79181) --- .../src/System/Threading/Interlocked.CoreCLR.cs | 2 ++ src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/emitxarch.cpp | 9 ++++++++- src/coreclr/jit/importercalls.cpp | 15 +++++++++++++-- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs index a4ab35444ae7..2e683dcf73c8 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs @@ -81,6 +81,7 @@ public static partial class Interlocked /// The value to which the parameter is set. /// The original value of . /// The address of location1 is a null pointer. + [Intrinsic] [MethodImpl(MethodImplOptions.InternalCall)] [return: NotNullIfNotNull(nameof(location1))] public static extern object? Exchange([NotNullIfNotNull(nameof(value))] ref object? location1, object? value); @@ -145,6 +146,7 @@ public static partial class Interlocked /// The object that is compared by reference to the object at . /// The original value in . /// The address of is a null pointer. + [Intrinsic] [MethodImpl(MethodImplOptions.InternalCall)] [return: NotNullIfNotNull(nameof(location1))] public static extern object? CompareExchange(ref object? location1, object? value, object? comparand); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 3adde7d491ac..ec9268597492 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4064,7 +4064,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* node) assert(addr->isUsedFromReg()); assert(data->isUsedFromReg()); - assert((size == EA_4BYTE) || (size == EA_PTRSIZE)); + assert((size == EA_4BYTE) || (size == EA_PTRSIZE) || (size == EA_GCREF)); genConsumeOperands(node); diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index a0008218ecf6..1c3c3bffd90a 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -12193,7 +12193,14 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) case IF_ARW_RRD: case IF_ARW_CNS: - assert(id->idGCref() == GCT_BYREF && (ins == INS_add || ins == INS_sub || ins == INS_sub_hide)); + if (id->idGCref() == GCT_BYREF) + { + assert(ins == INS_add || ins == INS_sub || ins == INS_sub_hide); + } + else + { + assert((id->idGCref() == GCT_GCREF) && (ins == INS_cmpxchg || ins == INS_xchg)); + } break; default: diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 90163e6f842f..3504e04d5025 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2967,7 +2967,12 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, { break; } - if ((retType != TYP_INT) && (retType != TYP_LONG)) + if ((retType == TYP_REF) && impStackTop(1).val->IsIntegralConst(0)) + { + // Intrinsify "object" overload in case of null assignment + // since we don't need the write barrier. + } + else if ((retType != TYP_INT) && (retType != TYP_LONG)) { break; } @@ -2997,7 +3002,13 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, { break; } - if ((retType != TYP_INT) && (retType != TYP_LONG)) + if ((retType == TYP_REF) && impStackTop().val->IsIntegralConst(0)) + { + // Intrinsify "object" overload in case of null assignment + // since we don't need the write barrier. + assert(ni == NI_System_Threading_Interlocked_Exchange); + } + else if ((retType != TYP_INT) && (retType != TYP_LONG)) { break; }