From de3cf791c18339e15ee994ce1ba7948f0d7c0cac Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 19 Dec 2020 17:03:58 +0300 Subject: [PATCH 01/12] Intrinsify Interlocked.And(Or) on arm64-v8.1 --- src/coreclr/jit/codegenarm64.cpp | 17 +++++++++-- src/coreclr/jit/codegenarmarch.cpp | 2 ++ src/coreclr/jit/codegenxarch.cpp | 5 ++++ src/coreclr/jit/decomposelongs.cpp | 2 ++ src/coreclr/jit/emitarm64.cpp | 2 ++ src/coreclr/jit/gentree.cpp | 4 ++- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/gtlist.h | 2 ++ src/coreclr/jit/importer.cpp | 30 +++++++++++++++++++ src/coreclr/jit/instrsarm64.h | 6 ++++ src/coreclr/jit/liveness.cpp | 4 +++ src/coreclr/jit/lower.cpp | 4 +++ src/coreclr/jit/lowerarmarch.cpp | 2 ++ src/coreclr/jit/lsraarm64.cpp | 2 ++ src/coreclr/jit/lsraxarch.cpp | 2 ++ src/coreclr/jit/namedintrinsiclist.h | 3 ++ src/coreclr/jit/optimizer.cpp | 4 ++- src/coreclr/jit/valuenum.cpp | 4 ++- .../src/System/Threading/Interlocked.cs | 4 +++ 19 files changed, 95 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 9bf02f45f8cd18..dde7ef1b69777b 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2786,10 +2786,10 @@ void CodeGen::genJumpTable(GenTree* treeNode) } //------------------------------------------------------------------------ -// genLockedInstructions: Generate code for a GT_XADD or GT_XCHG node. +// genLockedInstructions: Generate code for a GT_XADD, GT_XAND, GT_XXOR or GT_XCHG node. // // Arguments: -// treeNode - the GT_XADD/XCHG node +// treeNode - the GT_XADD/XAND/XXOR/XCHG node // void CodeGen::genLockedInstructions(GenTreeOp* treeNode) { @@ -2810,6 +2810,12 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) switch (treeNode->gtOper) { + case GT_XXOR: + GetEmitter()->emitIns_R_R_R(INS_ldsetal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, addrReg); + break; + case GT_XAND: + GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, addrReg); + break; case GT_XCHG: GetEmitter()->emitIns_R_R_R(INS_swpal, dataSize, dataReg, targetReg, addrReg); break; @@ -2831,6 +2837,9 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) } else { + // These are imported normally if Atomics aren't supported. + assert(!treeNode->OperIs(GT_XXOR, GT_XAND)); + regNumber exResultReg = treeNode->ExtractTempReg(RBM_ALLINT); regNumber storeDataReg = (treeNode->OperGet() == GT_XCHG) ? dataReg : treeNode->ExtractTempReg(RBM_ALLINT); regNumber loadReg = (targetReg != REG_NA) ? targetReg : storeDataReg; @@ -6221,6 +6230,10 @@ void CodeGen::genArm64EmitterUnitTests() theEmitter->emitIns_R_R_R(INS_ldadd, EA_8BYTE, REG_R8, REG_R9, REG_R10); theEmitter->emitIns_R_R_R(INS_ldadda, EA_8BYTE, REG_R8, REG_R9, REG_R10); theEmitter->emitIns_R_R_R(INS_ldaddal, EA_8BYTE, REG_R8, REG_R9, REG_R10); + theEmitter->emitIns_R_R_R(INS_ldclral, EA_4BYTE, REG_R8, REG_R9, REG_R10); + theEmitter->emitIns_R_R_R(INS_ldclral, EA_8BYTE, REG_R8, REG_R9, REG_R10); + theEmitter->emitIns_R_R_R(INS_ldsetal, EA_4BYTE, REG_R8, REG_R9, REG_R10); + theEmitter->emitIns_R_R_R(INS_ldsetal, EA_8BYTE, REG_R8, REG_R9, REG_R10); theEmitter->emitIns_R_R_R(INS_ldaddl, EA_8BYTE, REG_R8, REG_R9, REG_R10); theEmitter->emitIns_R_R_R(INS_swpb, EA_4BYTE, REG_R8, REG_R9, REG_R10); theEmitter->emitIns_R_R_R(INS_swpab, EA_4BYTE, REG_R8, REG_R9, REG_R10); diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 630cd8557cfcfc..a5cc53709cef51 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -407,6 +407,8 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef TARGET_ARM64 case GT_XCHG: + case GT_XXOR: + case GT_XAND: case GT_XADD: genLockedInstructions(treeNode->AsOp()); break; diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 648a11223c2e8d..627d8acf71adf0 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1706,6 +1706,11 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) genLockedInstructions(treeNode->AsOp()); break; + case GT_XXOR: + case GT_XAND: + NYI("Interlocked.Or and Interlocked.And aren't implemented for x86 yet."); + break; + case GT_MEMORYBARRIER: { CodeGen::BarrierKind barrierKind = diff --git a/src/coreclr/jit/decomposelongs.cpp b/src/coreclr/jit/decomposelongs.cpp index 33650892924a51..8e8ad265a47848 100644 --- a/src/coreclr/jit/decomposelongs.cpp +++ b/src/coreclr/jit/decomposelongs.cpp @@ -247,6 +247,8 @@ GenTree* DecomposeLongs::DecomposeNode(GenTree* tree) #endif // FEATURE_SIMD case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: case GT_CMPXCHG: diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp index 2f78d20a713967..650debea28f675 100644 --- a/src/coreclr/jit/emitarm64.cpp +++ b/src/coreclr/jit/emitarm64.cpp @@ -6091,6 +6091,8 @@ void emitter::emitIns_R_R_R( case INS_ldadda: case INS_ldaddal: case INS_ldaddl: + case INS_ldclral: + case INS_ldsetal: case INS_swpb: case INS_swpab: case INS_swpalb: diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 5b9a88c1d6a897..9694413e8416cc 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5474,7 +5474,7 @@ GenTree* GenTree::gtGetParent(GenTree*** parentChildPtrPtr) const bool GenTree::OperRequiresAsgFlag() { - if (OperIs(GT_ASG) || OperIs(GT_XADD, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) + if (OperIs(GT_ASG) || OperIs(GT_XADD, GT_XXOR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) { return true; } @@ -5549,6 +5549,8 @@ bool GenTree::OperIsImplicitIndir() const switch (gtOper) { case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: case GT_CMPXCHG: diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 165c98ce2471a7..9d24bb1c1fa619 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1519,7 +1519,7 @@ struct GenTree static bool OperIsAtomicOp(genTreeOps gtOper) { - return (gtOper == GT_XADD || gtOper == GT_XCHG || gtOper == GT_LOCKADD || gtOper == GT_CMPXCHG); + return (gtOper == GT_XADD || gtOper == GT_XXOR || gtOper == GT_XAND || gtOper == GT_XCHG || gtOper == GT_LOCKADD || gtOper == GT_CMPXCHG); } bool OperIsAtomicOp() const diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index f73f0c6ce68d71..ec3e6579492def 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -53,6 +53,8 @@ GTNODE(ARR_LENGTH , GenTreeArrLen ,0,(GTK_UNOP|GTK_EXOP)) // arr GTNODE(INTRINSIC , GenTreeIntrinsic ,0,(GTK_BINOP|GTK_EXOP)) // intrinsics GTNODE(LOCKADD , GenTreeOp ,0,(GTK_BINOP|GTK_NOVALUE)) +GTNODE(XAND , GenTreeOp ,0,GTK_BINOP) +GTNODE(XXOR , GenTreeOp ,0,GTK_BINOP) GTNODE(XADD , GenTreeOp ,0,GTK_BINOP) GTNODE(XCHG , GenTreeOp ,0,GTK_BINOP) GTNODE(CMPXCHG , GenTreeCmpXchg ,0,GTK_SPECIAL) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index aa75207070bd37..07dad090875504 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4337,6 +4337,25 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } +#ifdef TARGET_ARM64 + // Intrinsify Interlocked.Or and Interlocked.And only for arm64-v8.1 (and newer) + // TODO-CQ: Implement for XArch (https://github.com/dotnet/runtime/issues/32239). + case NI_System_Threading_Interlocked_Or: + case NI_System_Threading_Interlocked_And: + { + if (opts.OptimizationEnabled() && compOpportunisticallyDependsOn(InstructionSet_Atomics)) + { + assert(sig->numArgs == 2); + GenTree* op2 = impPopStack().val; + GenTree* op1 = impPopStack().val; + genTreeOps op = (ni == NI_System_Threading_Interlocked_Or) ? GT_XXOR : GT_XAND; + retNode = gtNewOperNode(op, genActualType(callType), op1, op2); + retNode->gtFlags |= GTF_GLOB_REF | GTF_ASG; + } + break; + } +#endif // TARGET_ARM64 + #ifdef FEATURE_HW_INTRINSICS case NI_System_Math_FusedMultiplyAdd: { @@ -4845,6 +4864,17 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) result = NI_System_Threading_Thread_get_ManagedThreadId; } } + else if (strcmp(className, "Interlocked") == 0) + { + if (strcmp(methodName, "And") == 0) + { + result = NI_System_Threading_Interlocked_And; + } + else if (strcmp(methodName, "Or") == 0) + { + result = NI_System_Threading_Interlocked_Or; + } + } } #if defined(TARGET_XARCH) || defined(TARGET_ARM64) else if (strcmp(namespaceName, "System.Buffers.Binary") == 0) diff --git a/src/coreclr/jit/instrsarm64.h b/src/coreclr/jit/instrsarm64.h index cddd9b0a986aa5..8f4d3b81a39437 100644 --- a/src/coreclr/jit/instrsarm64.h +++ b/src/coreclr/jit/instrsarm64.h @@ -1194,6 +1194,12 @@ INST1(ldadda, "ldadda", LD|ST, IF_LS_3E, 0xB8A00000) INST1(ldaddal, "ldaddal", LD|ST, IF_LS_3E, 0xB8E00000) // ldaddal Rm, Rt, [Xn] LS_3E 1X111000111mmmmm 000000nnnnnttttt B8E0 0000 Rm Rt Rn ARMv8.1 LSE Atomics +INST1(ldclral, "ldclral", LD|ST, IF_LS_3E, 0xB8E01000) + // ldclral Rm, Rt, [Xn] LS_3E 1X111000111mmmmm 000100nnnnnttttt B8E0 1000 Rm Rt Rn ARMv8.1 LSE Atomics + +INST1(ldsetal, "ldsetal", LD|ST, IF_LS_3E, 0xB8E03000) + // ldsetal Rm, Rt, [Xn] LS_3E 1X111000111mmmmm 001100nnnnnttttt B8E0 3000 Rm Rt Rn ARMv8.1 LSE Atomics + INST1(ldaddl, "ldaddl", LD|ST, IF_LS_3E, 0xB8600000) // ldaddl Rm, Rt, [Xn] LS_3E 1X111000011mmmmm 000000nnnnnttttt B860 0000 Rm Rt Rn ARMv8.1 LSE Atomics diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index d719970adeac9b..757878278c53d1 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -287,6 +287,8 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) // We'll assume these are use-then-defs of memory. case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: case GT_CMPXCHG: @@ -2052,6 +2054,8 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR break; case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: case GT_CMPXCHG: diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index d7488c85228757..b56d8dd46b4b57 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -318,10 +318,14 @@ GenTree* Lowering::LowerNode(GenTree* node) CheckImmedAndMakeContained(node, node->AsCmpXchg()->gtOpComparand); break; + case GT_XXOR: + case GT_XAND: case GT_XADD: CheckImmedAndMakeContained(node, node->AsOp()->gtOp2); break; #elif defined(TARGET_XARCH) + case GT_XXOR: + case GT_XAND: case GT_XADD: if (node->IsUnusedValue()) { diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 80e2bc2c5db81a..d0d6d3e8653c05 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -76,6 +76,8 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const #ifdef TARGET_ARM64 case GT_CMPXCHG: case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: return comp->compOpportunisticallyDependsOn(InstructionSet_Atomics) ? false diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 9463b1f4820870..070470f0f9c4be 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -425,6 +425,8 @@ int LinearScan::BuildNode(GenTree* tree) break; case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: { diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 133bcf28acf5a5..c07de8d83e8672 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -449,6 +449,8 @@ int LinearScan::BuildNode(GenTree* tree) } break; + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: { diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index c223307eebe9fe..7ee2c975bf81a7 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -53,6 +53,9 @@ enum NamedIntrinsic : unsigned short NI_IsSupported_Dynamic, NI_Throw_PlatformNotSupportedException, + NI_System_Threading_Interlocked_And, + NI_System_Threading_Interlocked_Or, + #ifdef FEATURE_HW_INTRINSICS NI_HW_INTRINSIC_START, #if defined(TARGET_XARCH) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index e19d7f903bb3e0..7d10a3247764a6 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7101,7 +7101,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack* blo m_beforeSideEffect = false; } } - else if (tree->OperIs(GT_XADD, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) + else if (tree->OperIs(GT_XADD, GT_XXOR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) { // If this node is a MEMORYBARRIER or an Atomic operation // then don't hoist and stop any further hoisting after this node @@ -7915,6 +7915,8 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) break; case GT_LOCKADD: + case GT_XXOR: + case GT_XAND: case GT_XADD: case GT_XCHG: case GT_CMPXCHG: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 447b3e3e6a428c..52ac5ea12d7522 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -5251,7 +5251,7 @@ void ValueNumStore::vnDumpSimdType(Compiler* comp, VNFuncApp* simdType) static UINT8 vnfOpAttribs[VNF_COUNT]; static genTreeOps genTreeOpsIllegalAsVNFunc[] = {GT_IND, // When we do heap memory. GT_NULLCHECK, GT_QMARK, GT_COLON, GT_LOCKADD, GT_XADD, GT_XCHG, - GT_CMPXCHG, GT_LCLHEAP, GT_BOX, + GT_CMPXCHG, GT_LCLHEAP, GT_BOX, GT_XXOR, GT_XAND, // These need special semantics: GT_COMMA, // == second argument (but with exception(s) from first). @@ -8232,6 +8232,8 @@ void Compiler::fgValueNumberTree(GenTree* tree) noway_assert("LOCKADD should not appear before lowering"); break; + case GT_XXOR: // Binop + case GT_XAND: // Binop case GT_XADD: // Binop case GT_XCHG: // Binop { diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs index 908182b9d1e0bd..f95b9ab590c35d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs @@ -133,6 +133,7 @@ public static ulong Read(ref ulong location) => /// The value to be combined with the integer at . /// The original value in . /// The address of is a null pointer. + [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int And(ref int location1, int value) { @@ -164,6 +165,7 @@ public static uint And(ref uint location1, uint value) => /// The value to be combined with the integer at . /// The original value in . /// The address of is a null pointer. + [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static long And(ref long location1, long value) { @@ -197,6 +199,7 @@ public static ulong And(ref ulong location1, ulong value) => /// The value to be combined with the integer at . /// The original value in . /// The address of is a null pointer. + [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int Or(ref int location1, int value) { @@ -228,6 +231,7 @@ public static uint Or(ref uint location1, uint value) => /// The value to be combined with the integer at . /// The original value in . /// The address of is a null pointer. + [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static long Or(ref long location1, long value) { From 4cf8a5d52a4cab277b83e2625c626fdec438168b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 19 Dec 2020 20:22:08 +0300 Subject: [PATCH 02/12] Formatting --- src/coreclr/jit/gentree.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 9d24bb1c1fa619..5174a13438b4ff 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1519,7 +1519,18 @@ struct GenTree static bool OperIsAtomicOp(genTreeOps gtOper) { - return (gtOper == GT_XADD || gtOper == GT_XXOR || gtOper == GT_XAND || gtOper == GT_XCHG || gtOper == GT_LOCKADD || gtOper == GT_CMPXCHG); + switch (oper) + { + case GT_XADD: + case GT_XXOR: + case GT_XAND: + case GT_XCHG: + case GT_LOCKADD: + case GT_CMPXCHG: + return true; + default: + return false; + } } bool OperIsAtomicOp() const From e22122d8499faaab6b0027876b642b9047f3b74d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 19 Dec 2020 20:37:19 +0300 Subject: [PATCH 03/12] Update gentree.h --- src/coreclr/jit/gentree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 5174a13438b4ff..58e8486783b87b 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1519,7 +1519,7 @@ struct GenTree static bool OperIsAtomicOp(genTreeOps gtOper) { - switch (oper) + switch (gtOper) { case GT_XADD: case GT_XXOR: From 83e4554194b5fe1f96c7a40f49ec39eccee656d1 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 20 Dec 2020 00:02:53 +0300 Subject: [PATCH 04/12] Add tests --- .../interlocked/and_or/add_or.csproj | 20 +++ .../threading/interlocked/and_or/and_or.cs | 121 ++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 src/tests/baseservices/threading/interlocked/and_or/add_or.csproj create mode 100644 src/tests/baseservices/threading/interlocked/and_or/and_or.cs diff --git a/src/tests/baseservices/threading/interlocked/and_or/add_or.csproj b/src/tests/baseservices/threading/interlocked/and_or/add_or.csproj new file mode 100644 index 00000000000000..78f0cd1cfe5cba --- /dev/null +++ b/src/tests/baseservices/threading/interlocked/and_or/add_or.csproj @@ -0,0 +1,20 @@ + + + Exe + + True + + + + + + + + + diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or.cs b/src/tests/baseservices/threading/interlocked/and_or/and_or.cs new file mode 100644 index 00000000000000..4d1f4738f7dae7 --- /dev/null +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or.cs @@ -0,0 +1,121 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using System.Threading; + +public class Program +{ + private static int s_RetCode = 100; + + public static int Main() + { + int[] testData = new int[] {int.MinValue, int.MinValue + 1, -1, 0, 1, 2, 1000, int.MaxValue - 1, int.MaxValue}; + for (int i = 0; i < testData.Length; i++) + { + for (int j = 0; j < testData.Length; j++) + { + int test1Value = testData[i]; + int test1Arg = testData[j]; + int ret1Value = RefImpl.XAnd32(ref test1Value, test1Arg); + + int test2Value = testData[i]; + int test2Arg = testData[j]; + int ret2Value = InterlockedImpl.XAnd32(ref test2Value, test2Arg); + AssertEquals(test1Value, test2Value); + AssertEquals(ret1Value, ret2Value); + + int test3Value = testData[i]; + int test3Arg = testData[j]; + RefImpl.XAnd32_noret(ref test3Value, test3Arg); + + int test4Value = testData[i]; + int test4Arg = testData[j]; + InterlockedImpl.XAnd32_noret(ref test4Value, test4Arg); + AssertEquals(test3Value, test4Value); + } + + ThrowsNRE(() => + { + ref int nullref = ref Unsafe.NullRef(); + InterlockedImpl.XAnd32(ref nullref, testData[i]); + }); + + ThrowsNRE(() => + { + ref int nullref = ref Unsafe.NullRef(); + InterlockedImpl.XAnd32_noret(ref nullref, testData[i]); + }); + + ThrowsNRE(() => + { + ref int nullref = ref Unsafe.NullRef(); + InterlockedImpl.XOr32(ref nullref, testData[i]); + }); + + ThrowsNRE(() => + { + ref int nullref = ref Unsafe.NullRef(); + InterlockedImpl.XOr32_noret(ref nullref, testData[i]); + }); + } + + + return s_RetCode; + } + + static void ThrowsNRE(Action action) + { + try + { + action(); + } + catch (NullReferenceException) + { + return; + } + + Console.WriteLine("ERROR: NullReferenceException was expected"); + s_RetCode++; + } + + static void AssertEquals(int expected, int actual, [CallerLineNumber] int line = 0) + { + if (expected != actual) + { + Console.WriteLine($"ERROR: {expected} != {actual} (Line:{line})"); + s_RetCode++; + } + } +} + +class RefImpl +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public static int XAnd32(ref int a, int b) { int src = a; a &= b; return src; } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XAnd32_noret(ref int a, int b) => a &= b; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int XOr32(ref int a, int b) { int src = a; a |= b; return src; } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XOr32_noret(ref int a, int b) => a |= b; +} + +class InterlockedImpl +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public static int XAnd32(ref int a, int b) => Interlocked.And(ref a, b); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XAnd32_noret(ref int a, int b) => Interlocked.And(ref a, b); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int XOr32(ref int a, int b) => Interlocked.Or(ref a, b); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XOr32_noret(ref int a, int b) => Interlocked.And(ref a, b); +} \ No newline at end of file From f451a2d644549f5b28943615977a5487f7f4f373 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 20 Dec 2020 00:03:29 +0300 Subject: [PATCH 05/12] Formatting --- src/coreclr/jit/codegenarm64.cpp | 6 ++++-- src/coreclr/jit/importer.cpp | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index dde7ef1b69777b..3b80ada8edc0a6 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2811,10 +2811,12 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) switch (treeNode->gtOper) { case GT_XXOR: - GetEmitter()->emitIns_R_R_R(INS_ldsetal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, addrReg); + GetEmitter()->emitIns_R_R_R(INS_ldsetal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, + addrReg); break; case GT_XAND: - GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, addrReg); + GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, + addrReg); break; case GT_XCHG: GetEmitter()->emitIns_R_R_R(INS_swpal, dataSize, dataReg, targetReg, addrReg); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 07dad090875504..feecd91846ce6e 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4349,7 +4349,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, GenTree* op2 = impPopStack().val; GenTree* op1 = impPopStack().val; genTreeOps op = (ni == NI_System_Threading_Interlocked_Or) ? GT_XXOR : GT_XAND; - retNode = gtNewOperNode(op, genActualType(callType), op1, op2); + retNode = gtNewOperNode(op, genActualType(callType), op1, op2); retNode->gtFlags |= GTF_GLOB_REF | GTF_ASG; } break; From 8ff1eadefa9070e2e31b21274dd0134736cc95b5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 20 Dec 2020 01:10:57 +0300 Subject: [PATCH 06/12] Add tests for int64 --- .../and_or/{and_or.cs => and_or_int32.cs} | 0 .../{add_or.csproj => and_or_int32.csproj} | 0 .../interlocked/and_or/and_or_int64.cs | 121 ++++++++++++++++++ .../interlocked/and_or/and_or_int64.csproj | 20 +++ 4 files changed, 141 insertions(+) rename src/tests/baseservices/threading/interlocked/and_or/{and_or.cs => and_or_int32.cs} (100%) rename src/tests/baseservices/threading/interlocked/and_or/{add_or.csproj => and_or_int32.csproj} (100%) create mode 100644 src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs create mode 100644 src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or.cs b/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.cs similarity index 100% rename from src/tests/baseservices/threading/interlocked/and_or/and_or.cs rename to src/tests/baseservices/threading/interlocked/and_or/and_or_int32.cs diff --git a/src/tests/baseservices/threading/interlocked/and_or/add_or.csproj b/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.csproj similarity index 100% rename from src/tests/baseservices/threading/interlocked/and_or/add_or.csproj rename to src/tests/baseservices/threading/interlocked/and_or/and_or_int32.csproj diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs new file mode 100644 index 00000000000000..4b2083f8c63491 --- /dev/null +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs @@ -0,0 +1,121 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using System.Threading; + +public class Program +{ + private static int s_RetCode = 100; + + public static int Main() + { + long[] testData = new long[] { long.MinValue, long.MinValue + 1, -1, 0, 1, 2, 1000, long.MaxValue - 1, long.MaxValue }; + for (long i = 0; i < testData.Length; i++) + { + for (long j = 0; j < testData.Length; j++) + { + long test1Value = testData[i]; + long test1Arg = testData[j]; + long ret1Value = RefImpl.XAnd32(ref test1Value, test1Arg); + + long test2Value = testData[i]; + long test2Arg = testData[j]; + long ret2Value = InterlockedImpl.XAnd32(ref test2Value, test2Arg); + AssertEquals(test1Value, test2Value); + AssertEquals(ret1Value, ret2Value); + + long test3Value = testData[i]; + long test3Arg = testData[j]; + RefImpl.XAnd32_noret(ref test3Value, test3Arg); + + long test4Value = testData[i]; + long test4Arg = testData[j]; + InterlockedImpl.XAnd32_noret(ref test4Value, test4Arg); + AssertEquals(test3Value, test4Value); + } + + ThrowsNRE(() => + { + ref long nullref = ref Unsafe.NullRef(); + InterlockedImpl.XAnd32(ref nullref, testData[i]); + }); + + ThrowsNRE(() => + { + ref long nullref = ref Unsafe.NullRef(); + InterlockedImpl.XAnd32_noret(ref nullref, testData[i]); + }); + + ThrowsNRE(() => + { + ref long nullref = ref Unsafe.NullRef(); + InterlockedImpl.XOr32(ref nullref, testData[i]); + }); + + ThrowsNRE(() => + { + ref long nullref = ref Unsafe.NullRef(); + InterlockedImpl.XOr32_noret(ref nullref, testData[i]); + }); + } + + + return s_RetCode; + } + + static void ThrowsNRE(Action action) + { + try + { + action(); + } + catch (NullReferenceException) + { + return; + } + + Console.WriteLine("ERROR: NullReferenceException was expected"); + s_RetCode++; + } + + static void AssertEquals(long expected, long actual, [CallerLineNumber] long line = 0) + { + if (expected != actual) + { + Console.WriteLine($"ERROR: {expected} != {actual} (Line:{line})"); + s_RetCode++; + } + } +} + +class RefImpl +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public static long XAnd32(ref long a, long b) { long src = a; a &= b; return src; } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XAnd32_noret(ref long a, long b) => a &= b; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static long XOr32(ref long a, long b) { long src = a; a |= b; return src; } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XOr32_noret(ref long a, long b) => a |= b; +} + +class InterlockedImpl +{ + [MethodImpl(MethodImplOptions.NoInlining)] + public static long XAnd32(ref long a, long b) => Interlocked.And(ref a, b); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XAnd32_noret(ref long a, long b) => Interlocked.And(ref a, b); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static long XOr32(ref long a, long b) => Interlocked.Or(ref a, b); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void XOr32_noret(ref long a, long b) => Interlocked.And(ref a, b); +} \ No newline at end of file diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj new file mode 100644 index 00000000000000..78f0cd1cfe5cba --- /dev/null +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj @@ -0,0 +1,20 @@ + + + Exe + + True + + + + + + + + + From 8436dc65bdbace75c1a6efeee8dac6159f6233ae Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 20 Dec 2020 19:05:59 +0300 Subject: [PATCH 07/12] Fix XAnd and the tests --- src/coreclr/jit/codegenarm64.cpp | 11 +++++--- src/coreclr/jit/lsraarm64.cpp | 5 ++++ .../interlocked/and_or/and_or_int32.cs | 27 +++++++++++++++++-- .../interlocked/and_or/and_or_int64.cs | 25 ++++++++++++++++- 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 3b80ada8edc0a6..3def537f5c057a 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2815,9 +2815,14 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) addrReg); break; case GT_XAND: - GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, - addrReg); - break; + { + // Grab a temp reg to perform `MVN` for dataReg first. + regNumber tempReg = treeNode->GetSingleTempReg(); + GetEmitter()->emitIns_R_R(INS_mvn, dataSize, tempReg, dataReg); + GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, tempReg, (targetReg == REG_NA) ? REG_ZR : targetReg, + addrReg); + break; + } case GT_XCHG: GetEmitter()->emitIns_R_R_R(INS_swpal, dataSize, dataReg, targetReg, addrReg); break; diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 070470f0f9c4be..f5f747c5242c8d 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -442,6 +442,11 @@ int LinearScan::BuildNode(GenTree* tree) buildInternalIntRegisterDefForNode(tree); } } + else if (tree->OperIs(GT_XAND)) + { + // for ldclral we need an internal register. + buildInternalIntRegisterDefForNode(tree); + } assert(!tree->gtGetOp1()->isContained()); RefPosition* op1Use = BuildUse(tree->gtGetOp1()); diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.cs b/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.cs index 4d1f4738f7dae7..a5f22074220703 100644 --- a/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.cs +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.cs @@ -11,11 +11,12 @@ public class Program public static int Main() { - int[] testData = new int[] {int.MinValue, int.MinValue + 1, -1, 0, 1, 2, 1000, int.MaxValue - 1, int.MaxValue}; + int[] testData = new int[] { int.MinValue, int.MinValue + 1, -1, 0, 1, 2, 1000, int.MaxValue - 1, int.MaxValue }; for (int i = 0; i < testData.Length; i++) { for (int j = 0; j < testData.Length; j++) { + // XAnd int test1Value = testData[i]; int test1Arg = testData[j]; int ret1Value = RefImpl.XAnd32(ref test1Value, test1Arg); @@ -26,6 +27,7 @@ public static int Main() AssertEquals(test1Value, test2Value); AssertEquals(ret1Value, ret2Value); + // XAnd_noret int test3Value = testData[i]; int test3Arg = testData[j]; RefImpl.XAnd32_noret(ref test3Value, test3Arg); @@ -34,6 +36,27 @@ public static int Main() int test4Arg = testData[j]; InterlockedImpl.XAnd32_noret(ref test4Value, test4Arg); AssertEquals(test3Value, test4Value); + + // XOr + int test5Value = testData[i]; + int test5Arg = testData[j]; + int ret5Value = RefImpl.XOr32(ref test5Value, test5Arg); + + int test6Value = testData[i]; + int test6Arg = testData[j]; + int ret6Value = InterlockedImpl.XOr32(ref test6Value, test6Arg); + AssertEquals(test5Value, test6Value); + AssertEquals(ret5Value, ret6Value); + + // XOr_noret + int test7Value = testData[i]; + int test7Arg = testData[j]; + RefImpl.XOr32_noret(ref test7Value, test7Arg); + + int test8Value = testData[i]; + int test8Arg = testData[j]; + InterlockedImpl.XOr32_noret(ref test8Value, test8Arg); + AssertEquals(test7Value, test8Value); } ThrowsNRE(() => @@ -117,5 +140,5 @@ class InterlockedImpl public static int XOr32(ref int a, int b) => Interlocked.Or(ref a, b); [MethodImpl(MethodImplOptions.NoInlining)] - public static void XOr32_noret(ref int a, int b) => Interlocked.And(ref a, b); + public static void XOr32_noret(ref int a, int b) => Interlocked.Or(ref a, b); } \ No newline at end of file diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs index 4b2083f8c63491..6432d72dc5e050 100644 --- a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs @@ -16,6 +16,7 @@ public static int Main() { for (long j = 0; j < testData.Length; j++) { + // XAnd long test1Value = testData[i]; long test1Arg = testData[j]; long ret1Value = RefImpl.XAnd32(ref test1Value, test1Arg); @@ -26,6 +27,7 @@ public static int Main() AssertEquals(test1Value, test2Value); AssertEquals(ret1Value, ret2Value); + // XAnd_noret long test3Value = testData[i]; long test3Arg = testData[j]; RefImpl.XAnd32_noret(ref test3Value, test3Arg); @@ -34,6 +36,27 @@ public static int Main() long test4Arg = testData[j]; InterlockedImpl.XAnd32_noret(ref test4Value, test4Arg); AssertEquals(test3Value, test4Value); + + // XOr + long test5Value = testData[i]; + long test5Arg = testData[j]; + long ret5Value = RefImpl.XOr32(ref test5Value, test5Arg); + + long test6Value = testData[i]; + long test6Arg = testData[j]; + long ret6Value = InterlockedImpl.XOr32(ref test6Value, test6Arg); + AssertEquals(test5Value, test6Value); + AssertEquals(ret5Value, ret6Value); + + // XOr_noret + long test7Value = testData[i]; + long test7Arg = testData[j]; + RefImpl.XOr32_noret(ref test7Value, test7Arg); + + long test8Value = testData[i]; + long test8Arg = testData[j]; + InterlockedImpl.XOr32_noret(ref test8Value, test8Arg); + AssertEquals(test7Value, test8Value); } ThrowsNRE(() => @@ -117,5 +140,5 @@ class InterlockedImpl public static long XOr32(ref long a, long b) => Interlocked.Or(ref a, b); [MethodImpl(MethodImplOptions.NoInlining)] - public static void XOr32_noret(ref long a, long b) => Interlocked.And(ref a, b); + public static void XOr32_noret(ref long a, long b) => Interlocked.Or(ref a, b); } \ No newline at end of file From 1cd25f52c39b87b0a910da3ce945e7605f8c9371 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 20 Dec 2020 19:16:41 +0300 Subject: [PATCH 08/12] Clean up --- .../interlocked/and_or/and_or_int64.cs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs index 6432d72dc5e050..01f7e014cff341 100644 --- a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.cs @@ -19,68 +19,68 @@ public static int Main() // XAnd long test1Value = testData[i]; long test1Arg = testData[j]; - long ret1Value = RefImpl.XAnd32(ref test1Value, test1Arg); + long ret1Value = RefImpl.XAnd64(ref test1Value, test1Arg); long test2Value = testData[i]; long test2Arg = testData[j]; - long ret2Value = InterlockedImpl.XAnd32(ref test2Value, test2Arg); + long ret2Value = InterlockedImpl.XAnd64(ref test2Value, test2Arg); AssertEquals(test1Value, test2Value); AssertEquals(ret1Value, ret2Value); // XAnd_noret long test3Value = testData[i]; long test3Arg = testData[j]; - RefImpl.XAnd32_noret(ref test3Value, test3Arg); + RefImpl.XAnd64_noret(ref test3Value, test3Arg); long test4Value = testData[i]; long test4Arg = testData[j]; - InterlockedImpl.XAnd32_noret(ref test4Value, test4Arg); + InterlockedImpl.XAnd64_noret(ref test4Value, test4Arg); AssertEquals(test3Value, test4Value); // XOr long test5Value = testData[i]; long test5Arg = testData[j]; - long ret5Value = RefImpl.XOr32(ref test5Value, test5Arg); + long ret5Value = RefImpl.XOr64(ref test5Value, test5Arg); long test6Value = testData[i]; long test6Arg = testData[j]; - long ret6Value = InterlockedImpl.XOr32(ref test6Value, test6Arg); + long ret6Value = InterlockedImpl.XOr64(ref test6Value, test6Arg); AssertEquals(test5Value, test6Value); AssertEquals(ret5Value, ret6Value); // XOr_noret long test7Value = testData[i]; long test7Arg = testData[j]; - RefImpl.XOr32_noret(ref test7Value, test7Arg); + RefImpl.XOr64_noret(ref test7Value, test7Arg); long test8Value = testData[i]; long test8Arg = testData[j]; - InterlockedImpl.XOr32_noret(ref test8Value, test8Arg); + InterlockedImpl.XOr64_noret(ref test8Value, test8Arg); AssertEquals(test7Value, test8Value); } ThrowsNRE(() => { ref long nullref = ref Unsafe.NullRef(); - InterlockedImpl.XAnd32(ref nullref, testData[i]); + InterlockedImpl.XAnd64(ref nullref, testData[i]); }); ThrowsNRE(() => { ref long nullref = ref Unsafe.NullRef(); - InterlockedImpl.XAnd32_noret(ref nullref, testData[i]); + InterlockedImpl.XAnd64_noret(ref nullref, testData[i]); }); ThrowsNRE(() => { ref long nullref = ref Unsafe.NullRef(); - InterlockedImpl.XOr32(ref nullref, testData[i]); + InterlockedImpl.XOr64(ref nullref, testData[i]); }); ThrowsNRE(() => { ref long nullref = ref Unsafe.NullRef(); - InterlockedImpl.XOr32_noret(ref nullref, testData[i]); + InterlockedImpl.XOr64_noret(ref nullref, testData[i]); }); } @@ -116,29 +116,29 @@ static void AssertEquals(long expected, long actual, [CallerLineNumber] long lin class RefImpl { [MethodImpl(MethodImplOptions.NoInlining)] - public static long XAnd32(ref long a, long b) { long src = a; a &= b; return src; } + public static long XAnd64(ref long a, long b) { long src = a; a &= b; return src; } [MethodImpl(MethodImplOptions.NoInlining)] - public static void XAnd32_noret(ref long a, long b) => a &= b; + public static void XAnd64_noret(ref long a, long b) => a &= b; [MethodImpl(MethodImplOptions.NoInlining)] - public static long XOr32(ref long a, long b) { long src = a; a |= b; return src; } + public static long XOr64(ref long a, long b) { long src = a; a |= b; return src; } [MethodImpl(MethodImplOptions.NoInlining)] - public static void XOr32_noret(ref long a, long b) => a |= b; + public static void XOr64_noret(ref long a, long b) => a |= b; } class InterlockedImpl { [MethodImpl(MethodImplOptions.NoInlining)] - public static long XAnd32(ref long a, long b) => Interlocked.And(ref a, b); + public static long XAnd64(ref long a, long b) => Interlocked.And(ref a, b); [MethodImpl(MethodImplOptions.NoInlining)] - public static void XAnd32_noret(ref long a, long b) => Interlocked.And(ref a, b); + public static void XAnd64_noret(ref long a, long b) => Interlocked.And(ref a, b); [MethodImpl(MethodImplOptions.NoInlining)] - public static long XOr32(ref long a, long b) => Interlocked.Or(ref a, b); + public static long XOr64(ref long a, long b) => Interlocked.Or(ref a, b); [MethodImpl(MethodImplOptions.NoInlining)] - public static void XOr32_noret(ref long a, long b) => Interlocked.Or(ref a, b); + public static void XOr64_noret(ref long a, long b) => Interlocked.Or(ref a, b); } \ No newline at end of file From 49590570e91c66f78969059bd778220db66f3307 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 20 Dec 2020 19:42:31 +0300 Subject: [PATCH 09/12] Formatting --- src/coreclr/jit/codegenarm64.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 3def537f5c057a..6cf78e28b87c54 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2815,14 +2815,14 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) addrReg); break; case GT_XAND: - { - // Grab a temp reg to perform `MVN` for dataReg first. - regNumber tempReg = treeNode->GetSingleTempReg(); - GetEmitter()->emitIns_R_R(INS_mvn, dataSize, tempReg, dataReg); - GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, tempReg, (targetReg == REG_NA) ? REG_ZR : targetReg, - addrReg); - break; - } + { + // Grab a temp reg to perform `MVN` for dataReg first. + regNumber tempReg = treeNode->GetSingleTempReg(); + GetEmitter()->emitIns_R_R(INS_mvn, dataSize, tempReg, dataReg); + GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, tempReg, (targetReg == REG_NA) ? REG_ZR : targetReg, + addrReg); + break; + } case GT_XCHG: GetEmitter()->emitIns_R_R_R(INS_swpal, dataSize, dataReg, targetReg, addrReg); break; From 6eae6938c44d3a38a6f99864f58dd99443f80310 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 20 Dec 2020 19:58:19 +0300 Subject: [PATCH 10/12] Update codegenarm64.cpp --- src/coreclr/jit/codegenarm64.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 6cf78e28b87c54..3befc841c907ab 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2820,7 +2820,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) regNumber tempReg = treeNode->GetSingleTempReg(); GetEmitter()->emitIns_R_R(INS_mvn, dataSize, tempReg, dataReg); GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, tempReg, (targetReg == REG_NA) ? REG_ZR : targetReg, - addrReg); + addrReg); break; } case GT_XCHG: From e76fe3d94c69b7e9d62c29876c41063f69c6a30c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 5 Feb 2021 14:51:50 +0300 Subject: [PATCH 11/12] Address feedback --- src/coreclr/jit/codegenarm64.cpp | 8 ++++---- src/coreclr/jit/gtlist.h | 2 +- src/coreclr/jit/importer.cpp | 5 ++++- src/coreclr/jit/liveness.cpp | 4 ++-- src/coreclr/jit/lower.cpp | 4 ++-- src/coreclr/jit/lowerarmarch.cpp | 2 +- src/coreclr/jit/lsraarm64.cpp | 2 +- src/coreclr/jit/lsraxarch.cpp | 2 +- src/coreclr/jit/optimizer.cpp | 4 ++-- src/coreclr/jit/valuenum.cpp | 4 ++-- .../threading/interlocked/and_or/and_or_int32.csproj | 10 ---------- .../threading/interlocked/and_or/and_or_int64.csproj | 10 ---------- 12 files changed, 20 insertions(+), 37 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 2ae770770b059b..a6de089a809648 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2787,10 +2787,10 @@ void CodeGen::genJumpTable(GenTree* treeNode) } //------------------------------------------------------------------------ -// genLockedInstructions: Generate code for a GT_XADD, GT_XAND, GT_XXOR or GT_XCHG node. +// genLockedInstructions: Generate code for a GT_XADD, GT_XAND, GT_XORR or GT_XCHG node. // // Arguments: -// treeNode - the GT_XADD/XAND/XXOR/XCHG node +// treeNode - the GT_XADD/XAND/XORR/XCHG node // void CodeGen::genLockedInstructions(GenTreeOp* treeNode) { @@ -2811,7 +2811,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) switch (treeNode->gtOper) { - case GT_XXOR: + case GT_XORR: GetEmitter()->emitIns_R_R_R(INS_ldsetal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg, addrReg); break; @@ -2838,7 +2838,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode) else { // These are imported normally if Atomics aren't supported. - assert(!treeNode->OperIs(GT_XXOR, GT_XAND)); + assert(!treeNode->OperIs(GT_XORR, GT_XAND)); regNumber exResultReg = treeNode->ExtractTempReg(RBM_ALLINT); regNumber storeDataReg = (treeNode->OperGet() == GT_XCHG) ? dataReg : treeNode->ExtractTempReg(RBM_ALLINT); diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index ec3e6579492def..638bbf3d39c128 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -54,7 +54,7 @@ GTNODE(INTRINSIC , GenTreeIntrinsic ,0,(GTK_BINOP|GTK_EXOP)) // int GTNODE(LOCKADD , GenTreeOp ,0,(GTK_BINOP|GTK_NOVALUE)) GTNODE(XAND , GenTreeOp ,0,GTK_BINOP) -GTNODE(XXOR , GenTreeOp ,0,GTK_BINOP) +GTNODE(XORR , GenTreeOp ,0,GTK_BINOP) GTNODE(XADD , GenTreeOp ,0,GTK_BINOP) GTNODE(XCHG , GenTreeOp ,0,GTK_BINOP) GTNODE(CMPXCHG , GenTreeCmpXchg ,0,GTK_SPECIAL) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 26917a9efd2b29..a8b0db1f04a39f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4348,7 +4348,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, assert(sig->numArgs == 2); GenTree* op2 = impPopStack().val; GenTree* op1 = impPopStack().val; - genTreeOps op = (ni == NI_System_Threading_Interlocked_Or) ? GT_XXOR : GT_XAND; + genTreeOps op = (ni == NI_System_Threading_Interlocked_Or) ? GT_XORR : GT_XAND; retNode = gtNewOperNode(op, genActualType(callType), op1, op2); retNode->gtFlags |= GTF_GLOB_REF | GTF_ASG; } @@ -4900,6 +4900,8 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) result = NI_System_Threading_Thread_get_ManagedThreadId; } } +#ifndef TARGET_ARM64 + // TODO-CQ: Implement for XArch (https://github.com/dotnet/runtime/issues/32239). else if (strcmp(className, "Interlocked") == 0) { if (strcmp(methodName, "And") == 0) @@ -4911,6 +4913,7 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) result = NI_System_Threading_Interlocked_Or; } } +#endif } #if defined(TARGET_XARCH) || defined(TARGET_ARM64) else if (strcmp(namespaceName, "System.Buffers.Binary") == 0) diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 757878278c53d1..cae25c9d593530 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -287,7 +287,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) // We'll assume these are use-then-defs of memory. case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: @@ -2054,7 +2054,7 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR break; case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 25309996f52e9c..4452e96225f1cc 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -318,13 +318,13 @@ GenTree* Lowering::LowerNode(GenTree* node) CheckImmedAndMakeContained(node, node->AsCmpXchg()->gtOpComparand); break; - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: CheckImmedAndMakeContained(node, node->AsOp()->gtOp2); break; #elif defined(TARGET_XARCH) - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: if (node->IsUnusedValue()) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index d0d6d3e8653c05..430d68094cbb65 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -76,7 +76,7 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const #ifdef TARGET_ARM64 case GT_CMPXCHG: case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: return comp->compOpportunisticallyDependsOn(InstructionSet_Atomics) diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index f5f747c5242c8d..d9aa71da582cf6 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -425,7 +425,7 @@ int LinearScan::BuildNode(GenTree* tree) break; case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 451be0c29f56f0..3872e794aa884e 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -449,7 +449,7 @@ int LinearScan::BuildNode(GenTree* tree) } break; - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 7a8619f0ea489b..5655b609f291eb 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7155,7 +7155,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack* blo m_beforeSideEffect = false; } } - else if (tree->OperIs(GT_XADD, GT_XXOR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) + else if (tree->OperIs(GT_XADD, GT_XORR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) { // If this node is a MEMORYBARRIER or an Atomic operation // then don't hoist and stop any further hoisting after this node @@ -7969,7 +7969,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) break; case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index cf259432fe8e3b..79c7d1d0620ef5 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -5516,7 +5516,7 @@ void ValueNumStore::vnDumpSimdType(Compiler* comp, VNFuncApp* simdType) static UINT8 vnfOpAttribs[VNF_COUNT]; static genTreeOps genTreeOpsIllegalAsVNFunc[] = {GT_IND, // When we do heap memory. GT_NULLCHECK, GT_QMARK, GT_COLON, GT_LOCKADD, GT_XADD, GT_XCHG, - GT_CMPXCHG, GT_LCLHEAP, GT_BOX, GT_XXOR, GT_XAND, + GT_CMPXCHG, GT_LCLHEAP, GT_BOX, GT_XORR, GT_XAND, // These need special semantics: GT_COMMA, // == second argument (but with exception(s) from first). @@ -8497,7 +8497,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) noway_assert("LOCKADD should not appear before lowering"); break; - case GT_XXOR: // Binop + case GT_XORR: // Binop case GT_XAND: // Binop case GT_XADD: // Binop case GT_XCHG: // Binop diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.csproj b/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.csproj index 78f0cd1cfe5cba..19781e26c20d81 100644 --- a/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.csproj +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int32.csproj @@ -7,14 +7,4 @@ - - - - diff --git a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj index 78f0cd1cfe5cba..19781e26c20d81 100644 --- a/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj +++ b/src/tests/baseservices/threading/interlocked/and_or/and_or_int64.csproj @@ -7,14 +7,4 @@ - - - - From c8c09e7921564325546b65fd518bdc6c87c9c874 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 5 Feb 2021 15:26:36 +0300 Subject: [PATCH 12/12] Address feedback --- src/coreclr/jit/codegenarmarch.cpp | 2 +- src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/decomposelongs.cpp | 2 +- src/coreclr/jit/gentree.cpp | 4 ++-- src/coreclr/jit/gentree.h | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 34b8e630ff3ee3..f250b88e9e0879 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -407,7 +407,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef TARGET_ARM64 case GT_XCHG: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: genLockedInstructions(treeNode->AsOp()); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 79a2e391dfa3e4..70d5ee00e63f20 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1735,7 +1735,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) genLockedInstructions(treeNode->AsOp()); break; - case GT_XXOR: + case GT_XORR: case GT_XAND: NYI("Interlocked.Or and Interlocked.And aren't implemented for x86 yet."); break; diff --git a/src/coreclr/jit/decomposelongs.cpp b/src/coreclr/jit/decomposelongs.cpp index 8e8ad265a47848..092e6b54fc654c 100644 --- a/src/coreclr/jit/decomposelongs.cpp +++ b/src/coreclr/jit/decomposelongs.cpp @@ -247,7 +247,7 @@ GenTree* DecomposeLongs::DecomposeNode(GenTree* tree) #endif // FEATURE_SIMD case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 75addc6da5d324..3d9803373ea13f 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5483,7 +5483,7 @@ GenTree* GenTree::gtGetParent(GenTree*** parentChildPtrPtr) const bool GenTree::OperRequiresAsgFlag() { - if (OperIs(GT_ASG) || OperIs(GT_XADD, GT_XXOR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) + if (OperIs(GT_ASG) || OperIs(GT_XADD, GT_XORR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER)) { return true; } @@ -5558,7 +5558,7 @@ bool GenTree::OperIsImplicitIndir() const switch (gtOper) { case GT_LOCKADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XADD: case GT_XCHG: diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index bbce5ae97744c7..8cf32cee6d546e 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1522,7 +1522,7 @@ struct GenTree switch (gtOper) { case GT_XADD: - case GT_XXOR: + case GT_XORR: case GT_XAND: case GT_XCHG: case GT_LOCKADD: