diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 092a031f270480..1bacf5ffd61abe 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3930,9 +3930,9 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) { assert(treeNode->OperIs(GT_CMPXCHG)); - GenTree* addr = treeNode->gtOpLocation; // arg1 - GenTree* data = treeNode->gtOpValue; // arg2 - GenTree* comparand = treeNode->gtOpComparand; // arg3 + GenTree* addr = treeNode->Addr(); // arg1 + GenTree* data = treeNode->Data(); // arg2 + GenTree* comparand = treeNode->Comparand(); // arg3 regNumber targetReg = treeNode->GetRegNum(); regNumber dataReg = data->GetRegNum(); diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 4a64ebb374a19f..81c90fe0937985 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -2670,9 +2670,9 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) { assert(treeNode->OperIs(GT_CMPXCHG)); - GenTree* locOp = treeNode->gtOpLocation; - GenTree* valOp = treeNode->gtOpValue; - GenTree* comparandOp = treeNode->gtOpComparand; + GenTree* locOp = treeNode->Addr(); + GenTree* valOp = treeNode->Data(); + GenTree* comparandOp = treeNode->Comparand(); regNumber target = treeNode->GetRegNum(); regNumber loc = locOp->GetRegNum(); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 16ffe5a8d77117..a73d96ba10d72d 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4396,9 +4396,9 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* tree) var_types targetType = tree->TypeGet(); regNumber targetReg = tree->GetRegNum(); - GenTree* location = tree->gtOpLocation; // arg1 - GenTree* value = tree->gtOpValue; // arg2 - GenTree* comparand = tree->gtOpComparand; // arg3 + GenTree* location = tree->Addr(); // arg1 + GenTree* value = tree->Data(); // arg2 + GenTree* comparand = tree->Comparand(); // arg3 assert(location->GetRegNum() != REG_NA && location->GetRegNum() != REG_RAX); assert(value->GetRegNum() != REG_NA && value->GetRegNum() != REG_RAX); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b5c08a34c30115..43cd60212c7dd9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2899,6 +2899,9 @@ class Compiler var_types gtTypeForNullCheck(GenTree* tree); void gtChangeOperToNullCheck(GenTree* tree, BasicBlock* block); + GenTree* gtNewAtomicNode( + genTreeOps oper, var_types type, GenTree* addr, GenTree* value, GenTree* comparand = nullptr); + GenTree* gtNewTempStore(unsigned tmp, GenTree* val, unsigned curLevel = CHECK_SPILL_NONE, @@ -11263,17 +11266,17 @@ class GenTreeVisitor { GenTreeCmpXchg* const cmpXchg = node->AsCmpXchg(); - result = WalkTree(&cmpXchg->gtOpLocation, cmpXchg); + result = WalkTree(&cmpXchg->Addr(), cmpXchg); if (result == fgWalkResult::WALK_ABORT) { return result; } - result = WalkTree(&cmpXchg->gtOpValue, cmpXchg); + result = WalkTree(&cmpXchg->Data(), cmpXchg); if (result == fgWalkResult::WALK_ABORT) { return result; } - result = WalkTree(&cmpXchg->gtOpComparand, cmpXchg); + result = WalkTree(&cmpXchg->Comparand(), cmpXchg); if (result == fgWalkResult::WALK_ABORT) { return result; diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index a786b56edc29dc..5f150b20b0d4aa 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4656,15 +4656,15 @@ void GenTree::VisitOperands(TVisitor visitor) case GT_CMPXCHG: { GenTreeCmpXchg* const cmpXchg = this->AsCmpXchg(); - if (visitor(cmpXchg->gtOpLocation) == VisitResult::Abort) + if (visitor(cmpXchg->Addr()) == VisitResult::Abort) { return; } - if (visitor(cmpXchg->gtOpValue) == VisitResult::Abort) + if (visitor(cmpXchg->Data()) == VisitResult::Abort) { return; } - visitor(cmpXchg->gtOpComparand); + visitor(cmpXchg->Comparand()); return; } diff --git a/src/coreclr/jit/earlyprop.cpp b/src/coreclr/jit/earlyprop.cpp index b422c79a37ba36..7f9f5153dbde3c 100644 --- a/src/coreclr/jit/earlyprop.cpp +++ b/src/coreclr/jit/earlyprop.cpp @@ -175,7 +175,7 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck optPropKind propKind = optPropKind::OPK_INVALID; bool folded = false; - if (tree->OperIsIndirOrArrMetaData()) + if (tree->OperIsIndirOrArrMetaData() && !tree->OperIsAtomicZeroDiffQuirk()) { // optFoldNullCheck takes care of updating statement info if a null check is removed. folded = optFoldNullCheck(tree, nullCheckMap); diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 16d0b0e307010b..b8d84fdbdd13d3 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -783,7 +783,7 @@ GenTree* BlockCountInstrumentor::CreateCounterIncrement(Compiler* comp, uint8_t* GenTree* addressNode = comp->gtNewIconHandleNode(reinterpret_cast(counterAddr), GTF_ICON_BBC_PTR); // Interlocked increment - result = comp->gtNewOperNode(GT_XADD, countType, addressNode, comp->gtNewIconNode(1, countType)); + result = comp->gtNewAtomicNode(GT_XADD, countType, addressNode, comp->gtNewIconNode(1, countType)); } if (scalable) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ec9ef52362968d..1d78ccbf852f3e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2994,9 +2994,9 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) return GenTreeFieldList::Equals(op1->AsFieldList(), op2->AsFieldList()); case GT_CMPXCHG: - return Compare(op1->AsCmpXchg()->gtOpLocation, op2->AsCmpXchg()->gtOpLocation) && - Compare(op1->AsCmpXchg()->gtOpValue, op2->AsCmpXchg()->gtOpValue) && - Compare(op1->AsCmpXchg()->gtOpComparand, op2->AsCmpXchg()->gtOpComparand); + return Compare(op1->AsCmpXchg()->Addr(), op2->AsCmpXchg()->Addr()) && + Compare(op1->AsCmpXchg()->Data(), op2->AsCmpXchg()->Data()) && + Compare(op1->AsCmpXchg()->Comparand(), op2->AsCmpXchg()->Comparand()); case GT_STORE_DYN_BLK: return Compare(op1->AsStoreDynBlk()->Addr(), op2->AsStoreDynBlk()->Addr()) && @@ -3546,9 +3546,9 @@ unsigned Compiler::gtHashValue(GenTree* tree) break; case GT_CMPXCHG: - hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpLocation)); - hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpValue)); - hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpComparand)); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Addr())); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Data())); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Comparand())); break; case GT_STORE_DYN_BLK: @@ -6203,27 +6203,31 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) break; case GT_CMPXCHG: + { + GenTree* addr = tree->AsCmpXchg()->Addr(); + level = gtSetEvalOrder(addr); + costSz = addr->GetCostSz(); - level = gtSetEvalOrder(tree->AsCmpXchg()->gtOpLocation); - costSz = tree->AsCmpXchg()->gtOpLocation->GetCostSz(); - - lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->gtOpValue); + GenTree* value = tree->AsCmpXchg()->Data(); + lvl2 = gtSetEvalOrder(value); if (level < lvl2) { level = lvl2; } - costSz += tree->AsCmpXchg()->gtOpValue->GetCostSz(); + costSz += value->GetCostSz(); - lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->gtOpComparand); + GenTree* comparand = tree->AsCmpXchg()->Comparand(); + lvl2 = gtSetEvalOrder(comparand); if (level < lvl2) { level = lvl2; } - costSz += tree->AsCmpXchg()->gtOpComparand->GetCostSz(); + costSz += comparand->GetCostSz(); costEx = MAX_COST; // Seriously, what could be more expensive than lock cmpxchg? costSz += 5; // size of lock cmpxchg [reg+C], reg - break; + } + break; case GT_STORE_DYN_BLK: level = gtSetEvalOrder(tree->AsStoreDynBlk()->Addr()); @@ -6522,19 +6526,19 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) case GT_CMPXCHG: { GenTreeCmpXchg* const cmpXchg = this->AsCmpXchg(); - if (operand == cmpXchg->gtOpLocation) + if (operand == cmpXchg->Addr()) { - *pUse = &cmpXchg->gtOpLocation; + *pUse = &cmpXchg->Addr(); return true; } - if (operand == cmpXchg->gtOpValue) + if (operand == cmpXchg->Data()) { - *pUse = &cmpXchg->gtOpValue; + *pUse = &cmpXchg->Data(); return true; } - if (operand == cmpXchg->gtOpComparand) + if (operand == cmpXchg->Comparand()) { - *pUse = &cmpXchg->gtOpComparand; + *pUse = &cmpXchg->Comparand(); return true; } return false; @@ -6999,7 +7003,7 @@ ExceptionSetFlags GenTree::OperExceptions(Compiler* comp) #endif // FEATURE_HW_INTRINSICS default: - assert(!OperMayOverflow() && !OperIsIndirOrArrMetaData()); + assert(!OperMayOverflow() && (!OperIsIndirOrArrMetaData() || OperIsAtomicZeroDiffQuirk())); return ExceptionSetFlags::None; } } @@ -8344,6 +8348,38 @@ GenTree* Compiler::gtNewStoreValueNode( return store; } +//------------------------------------------------------------------------ +// gtNewAtomicNode: Create a new atomic operation node. +// +// Arguments: +// oper - The atomic oper +// type - Type to store/load +// addr - Destination ("location") address +// value - Value +// comparand - Comparand value for a CMPXCHG +// +// Return Value: +// The created node. +// +GenTree* Compiler::gtNewAtomicNode(genTreeOps oper, var_types type, GenTree* addr, GenTree* value, GenTree* comparand) +{ + assert(GenTree::OperIsAtomicOp(oper) && ((oper == GT_CMPXCHG) == (comparand != nullptr))); + GenTree* node; + if (comparand != nullptr) + { + node = new (this, GT_CMPXCHG) GenTreeCmpXchg(type, addr, value, comparand); + addr->gtFlags |= GTF_DONT_CSE; + } + else + { + node = new (this, oper) GenTreeIndir(oper, type, addr, value); + } + + // All atomics are opaque global stores. + node->AddAllEffectsFlags(GTF_ASG | GTF_GLOB_REF); + return node; +} + //------------------------------------------------------------------------ // FixupInitBlkValue: Fixup the init value for an initBlk operation // @@ -9333,9 +9369,9 @@ GenTree* Compiler::gtCloneExpr( case GT_CMPXCHG: copy = new (this, GT_CMPXCHG) GenTreeCmpXchg(tree->TypeGet(), - gtCloneExpr(tree->AsCmpXchg()->gtOpLocation, addFlags, deepVarNum, deepVarVal), - gtCloneExpr(tree->AsCmpXchg()->gtOpValue, addFlags, deepVarNum, deepVarVal), - gtCloneExpr(tree->AsCmpXchg()->gtOpComparand, addFlags, deepVarNum, deepVarVal)); + gtCloneExpr(tree->AsCmpXchg()->Addr(), addFlags, deepVarNum, deepVarVal), + gtCloneExpr(tree->AsCmpXchg()->Data(), addFlags, deepVarNum, deepVarVal), + gtCloneExpr(tree->AsCmpXchg()->Comparand(), addFlags, deepVarNum, deepVarVal)); break; case GT_STORE_DYN_BLK: @@ -9977,7 +10013,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) return; case GT_CMPXCHG: - m_edge = &m_node->AsCmpXchg()->gtOpLocation; + m_edge = &m_node->AsCmpXchg()->Addr(); assert(*m_edge != nullptr); m_advance = &GenTreeUseEdgeIterator::AdvanceCmpXchg; return; @@ -10026,11 +10062,11 @@ void GenTreeUseEdgeIterator::AdvanceCmpXchg() switch (m_state) { case 0: - m_edge = &m_node->AsCmpXchg()->gtOpValue; + m_edge = &m_node->AsCmpXchg()->Data(); m_state = 1; break; case 1: - m_edge = &m_node->AsCmpXchg()->gtOpComparand; + m_edge = &m_node->AsCmpXchg()->Comparand(); m_advance = &GenTreeUseEdgeIterator::Terminate; break; default: @@ -12629,9 +12665,9 @@ void Compiler::gtDispTree(GenTree* tree, if (!topOnly) { - gtDispChild(tree->AsCmpXchg()->gtOpLocation, indentStack, IIArc, nullptr, topOnly); - gtDispChild(tree->AsCmpXchg()->gtOpValue, indentStack, IIArc, nullptr, topOnly); - gtDispChild(tree->AsCmpXchg()->gtOpComparand, indentStack, IIArcBottom, nullptr, topOnly); + gtDispChild(tree->AsCmpXchg()->Addr(), indentStack, IIArc, nullptr, topOnly); + gtDispChild(tree->AsCmpXchg()->Data(), indentStack, IIArc, nullptr, topOnly); + gtDispChild(tree->AsCmpXchg()->Comparand(), indentStack, IIArcBottom, nullptr, topOnly); } break; @@ -17489,7 +17525,7 @@ GenTreeLclVar* GenTree::IsImplicitByrefParameterValuePostMorph(Compiler* compile { #if FEATURE_IMPLICIT_BYREFS && !defined(TARGET_LOONGARCH64) // TODO-LOONGARCH64-CQ: enable this. - if (!OperIsIndir()) + if (!OperIsLoad()) { return nullptr; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 3605f3bcd33262..c7d96020b64267 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1536,8 +1536,9 @@ struct GenTree // OperIsIndir() returns true also for indirection nodes such as GT_BLK, etc. as well as GT_NULLCHECK. static bool OperIsIndir(genTreeOps gtOper) { - static_assert_no_msg(AreContiguous(GT_IND, GT_STOREIND, GT_BLK, GT_STORE_BLK, GT_STORE_DYN_BLK, GT_NULLCHECK)); - return (GT_IND <= gtOper) && (gtOper <= GT_NULLCHECK); + static_assert_no_msg(AreContiguous(GT_LOCKADD, GT_XAND, GT_XORR, GT_XADD, GT_XCHG, GT_CMPXCHG, GT_IND, + GT_STOREIND, GT_BLK, GT_STORE_BLK, GT_STORE_DYN_BLK, GT_NULLCHECK)); + return (GT_LOCKADD <= gtOper) && (gtOper <= GT_NULLCHECK); } static bool OperIsArrLength(genTreeOps gtOper) @@ -1612,6 +1613,22 @@ struct GenTree return OperIsAtomicOp(gtOper); } + bool OperIsAtomicZeroDiffQuirk() const + { + // TODO-Cleanup: delete. + return OperIsAtomicOp(); + } + + static bool OperIsLoad(genTreeOps gtOper) + { + return (gtOper == GT_IND) || (gtOper == GT_BLK); + } + + bool OperIsLoad() const + { + return OperIsLoad(gtOper); + } + static bool OperIsStore(genTreeOps gtOper) { return (OperKind(gtOper) & GTK_STORE) != 0; @@ -5617,31 +5634,6 @@ struct GenTreeCall final : public GenTree #endif }; -struct GenTreeCmpXchg : public GenTree -{ - GenTree* gtOpLocation; - GenTree* gtOpValue; - GenTree* gtOpComparand; - - GenTreeCmpXchg(var_types type, GenTree* loc, GenTree* val, GenTree* comparand) - : GenTree(GT_CMPXCHG, type), gtOpLocation(loc), gtOpValue(val), gtOpComparand(comparand) - { - // There's no reason to do a compare-exchange on a local location, so we'll assume that all of these - // have global effects. - gtFlags |= (GTF_GLOB_REF | GTF_ASG); - - // Merge in flags from operands - gtFlags |= gtOpLocation->gtFlags & GTF_ALL_EFFECT; - gtFlags |= gtOpValue->gtFlags & GTF_ALL_EFFECT; - gtFlags |= gtOpComparand->gtFlags & GTF_ALL_EFFECT; - } -#if DEBUGGABLE_GENTREE - GenTreeCmpXchg() : GenTree() - { - } -#endif -}; - #if !defined(TARGET_64BIT) struct GenTreeMultiRegOp : public GenTreeOp { @@ -7148,7 +7140,7 @@ struct GenTreeIndir : public GenTreeOp GenTree*& Data() { - assert(OperIs(GT_STOREIND) || OperIsStoreBlk()); + assert(OperIs(GT_STOREIND) || OperIsStoreBlk() || OperIsAtomicOp()); return gtOp2; } @@ -7442,6 +7434,30 @@ struct GenTreeStoreInd : public GenTreeIndir #endif }; +struct GenTreeCmpXchg : public GenTreeIndir +{ +private: + GenTree* m_comparand; + +public: + GenTreeCmpXchg(var_types type, GenTree* loc, GenTree* val, GenTree* comparand) + : GenTreeIndir(GT_CMPXCHG, type, loc, val), m_comparand(comparand) + { + gtFlags |= comparand->gtFlags & GTF_ALL_EFFECT; + } + +#if DEBUGGABLE_GENTREE + GenTreeCmpXchg() : GenTreeIndir() + { + } +#endif + + GenTree*& Comparand() + { + return m_comparand; + } +}; + /* gtRetExp -- Place holder for the return expression from an inline candidate (GT_RET_EXPR) */ struct GenTreeRetExpr : public GenTree diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index b3a3d2dbb470c9..e88c447d3774db 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -57,15 +57,6 @@ GTNODE(NOP , GenTree ,0,0,GTK_UNOP|DBK_NOCONTAIN) GTNODE(NEG , GenTreeOp ,0,0,GTK_UNOP) GTNODE(INTRINSIC , GenTreeIntrinsic ,0,0,GTK_BINOP|GTK_EXOP) - -GTNODE(LOCKADD , GenTreeOp ,0,1,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR) -GTNODE(XAND , GenTreeOp ,0,1,GTK_BINOP) -GTNODE(XORR , GenTreeOp ,0,1,GTK_BINOP) -GTNODE(XADD , GenTreeOp ,0,1,GTK_BINOP) -GTNODE(XCHG , GenTreeOp ,0,1,GTK_BINOP) -GTNODE(CMPXCHG , GenTreeCmpXchg ,0,1,GTK_SPECIAL) -GTNODE(MEMORYBARRIER , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE) - GTNODE(KEEPALIVE , GenTree ,0,0,GTK_UNOP|GTK_NOVALUE) // keep operand alive, generate no code, produce no result GTNODE(CAST , GenTreeCast ,0,0,GTK_UNOP|GTK_EXOP) // conversion to another type @@ -79,6 +70,14 @@ GTNODE(LCLHEAP , GenTreeOp ,0,1,GTK_UNOP|DBK_NOCONTAIN) // all GTNODE(BOUNDS_CHECK , GenTreeBoundsChk ,0,1,GTK_BINOP|GTK_EXOP|GTK_NOVALUE) // a bounds check - for arrays/spans/SIMDs/HWINTRINSICs +GTNODE(MEMORYBARRIER , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE) +GTNODE(LOCKADD , GenTreeOp ,0,1,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR) +GTNODE(XAND , GenTreeOp ,0,1,GTK_BINOP) +GTNODE(XORR , GenTreeOp ,0,1,GTK_BINOP) +GTNODE(XADD , GenTreeOp ,0,1,GTK_BINOP) +GTNODE(XCHG , GenTreeOp ,0,1,GTK_BINOP) +GTNODE(CMPXCHG , GenTreeCmpXchg ,0,1,GTK_SPECIAL) + GTNODE(IND , GenTreeIndir ,0,1,GTK_UNOP) // Load indirection GTNODE(STOREIND , GenTreeStoreInd ,0,1,GTK_BINOP|GTK_EXOP|GTK_NOVALUE|GTK_STORE) // Store indirection GTNODE(BLK , GenTreeBlk ,0,1,GTK_UNOP|GTK_EXOP) // Struct load diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h index 317e0b3f071b77..c53eff1bbf36b8 100644 --- a/src/coreclr/jit/gtstructs.h +++ b/src/coreclr/jit/gtstructs.h @@ -85,15 +85,15 @@ GTSTRUCT_1(RetExpr , GT_RET_EXPR) GTSTRUCT_1(ILOffset , GT_IL_OFFSET) GTSTRUCT_2(CopyOrReload, GT_COPY, GT_RELOAD) GTSTRUCT_1(ClsVar , GT_CLS_VAR_ADDR) -GTSTRUCT_1(CmpXchg , GT_CMPXCHG) GTSTRUCT_1(AddrMode , GT_LEA) -GTSTRUCT_N(Blk , GT_BLK, GT_STORE_BLK, GT_STORE_DYN_BLK) -GTSTRUCT_1(StoreDynBlk , GT_STORE_DYN_BLK) GTSTRUCT_1(Qmark , GT_QMARK) GTSTRUCT_1(PhiArg , GT_PHI_ARG) GTSTRUCT_1(Phi , GT_PHI) +GTSTRUCT_N(Indir , GT_IND, GT_NULLCHECK, GT_BLK, GT_STORE_BLK, GT_STORE_DYN_BLK, GT_LOCKADD, GT_XAND, GT_XORR, GT_XADD, GT_XCHG, GT_CMPXCHG, GT_STOREIND) +GTSTRUCT_N(Blk , GT_BLK, GT_STORE_BLK, GT_STORE_DYN_BLK) +GTSTRUCT_1(StoreDynBlk , GT_STORE_DYN_BLK) GTSTRUCT_1(StoreInd , GT_STOREIND) -GTSTRUCT_N(Indir , GT_STOREIND, GT_IND, GT_NULLCHECK, GT_BLK, GT_STORE_BLK, GT_STORE_DYN_BLK) +GTSTRUCT_1(CmpXchg , GT_CMPXCHG) #ifdef TARGET_ARM64 GTSTRUCT_N(Conditional , GT_SELECT, GT_SELECT_INC, GT_SELECT_INV, GT_SELECT_NEG) #else diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index d340354d34ef1f..d920e4143ceefb 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3239,8 +3239,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, GenTree* op2 = impPopStack().val; GenTree* op1 = impPopStack().val; 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; + retNode = gtNewAtomicNode(op, genActualType(callType), op1, op2); } break; } @@ -3271,11 +3270,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, GenTree* op3 = impPopStack().val; // comparand GenTree* op2 = impPopStack().val; // value GenTree* op1 = impPopStack().val; // location - - GenTree* node = new (this, GT_CMPXCHG) GenTreeCmpXchg(genActualType(callType), op1, op2, op3); - - node->AsCmpXchg()->gtOpLocation->gtFlags |= GTF_DONT_CSE; - retNode = node; + retNode = gtNewAtomicNode(GT_CMPXCHG, genActualType(callType), op1, op2, op3); break; } @@ -3305,19 +3300,12 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, GenTree* op1 = impPopStack().val; // This creates: - // val // XAdd - // addr - // field (for example) + // val + // field_addr (for example) // - // In the case where the first argument is the address of a local, we might - // want to make this *not* make the var address-taken -- but atomic instructions - // on a local are probably pretty useless anyway, so we probably don't care. - - op1 = gtNewOperNode(ni == NI_System_Threading_Interlocked_ExchangeAdd ? GT_XADD : GT_XCHG, - genActualType(callType), op1, op2); - op1->gtFlags |= GTF_GLOB_REF | GTF_ASG; - retNode = op1; + retNode = gtNewAtomicNode(ni == NI_System_Threading_Interlocked_ExchangeAdd ? GT_XADD : GT_XCHG, + genActualType(callType), op1, op2); break; } #endif // defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_RISCV64) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index bb31df9240a57a..de70ef47f3eebf 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -637,7 +637,7 @@ GenTree* Lowering::LowerNode(GenTree* node) #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) case GT_CMPXCHG: - CheckImmedAndMakeContained(node, node->AsCmpXchg()->gtOpComparand); + CheckImmedAndMakeContained(node, node->AsCmpXchg()->Comparand()); break; case GT_XORR: @@ -6304,10 +6304,10 @@ GenTree* Lowering::LowerAdd(GenTreeOp* node) #ifdef TARGET_XARCH if (BlockRange().TryGetUse(node, &use)) { - // If this is a child of an indir, let the parent handle it. + // If this is a child of an ordinary indir, let the parent handle it. // If there is a chain of adds, only look at the topmost one. GenTree* parent = use.User(); - if (!parent->OperIsIndir() && !parent->OperIs(GT_ADD)) + if ((!parent->OperIsIndir() || parent->OperIsAtomicOp()) && !parent->OperIs(GT_ADD)) { TryCreateAddrMode(node, false, parent); } diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index d3bba13f8ea65d..0064743e7ea193 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -960,7 +960,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_CMPXCHG: { GenTreeCmpXchg* cmpXchgNode = tree->AsCmpXchg(); - srcCount = cmpXchgNode->gtOpComparand->isContained() ? 2 : 3; + srcCount = cmpXchgNode->Comparand()->isContained() ? 2 : 3; assert(dstCount == 1); if (!compiler->compOpportunisticallyDependsOn(InstructionSet_Atomics)) @@ -975,13 +975,13 @@ int LinearScan::BuildNode(GenTree* tree) // For ARMv8.1 atomic cas the lifetime of the addr and data must be extended to prevent // them being reused as the target register which must be destroyed early - RefPosition* locationUse = BuildUse(tree->AsCmpXchg()->gtOpLocation); + RefPosition* locationUse = BuildUse(tree->AsCmpXchg()->Addr()); setDelayFree(locationUse); - RefPosition* valueUse = BuildUse(tree->AsCmpXchg()->gtOpValue); + RefPosition* valueUse = BuildUse(tree->AsCmpXchg()->Data()); setDelayFree(valueUse); - if (!cmpXchgNode->gtOpComparand->isContained()) + if (!cmpXchgNode->Comparand()->isContained()) { - RefPosition* comparandUse = BuildUse(tree->AsCmpXchg()->gtOpComparand); + RefPosition* comparandUse = BuildUse(tree->AsCmpXchg()->Comparand()); // For ARMv8 exclusives the lifetime of the comparand must be extended because // it may be used used multiple during retries diff --git a/src/coreclr/jit/lsrariscv64.cpp b/src/coreclr/jit/lsrariscv64.cpp index 117a46f71fad25..a9c357aca8ac09 100644 --- a/src/coreclr/jit/lsrariscv64.cpp +++ b/src/coreclr/jit/lsrariscv64.cpp @@ -369,15 +369,15 @@ int LinearScan::BuildNode(GenTree* tree) case GT_CMPXCHG: { GenTreeCmpXchg* cas = tree->AsCmpXchg(); - assert(!cas->gtOpComparand->isContained()); + assert(!cas->Comparand()->isContained()); srcCount = 3; assert(dstCount == 1); buildInternalIntRegisterDefForNode(tree); // temp reg for store conditional error // Extend lifetimes of argument regs because they may be reused during retries - setDelayFree(BuildUse(cas->gtOpLocation)); - setDelayFree(BuildUse(cas->gtOpValue)); - setDelayFree(BuildUse(cas->gtOpComparand)); + setDelayFree(BuildUse(cas->Addr())); + setDelayFree(BuildUse(cas->Data())); + setDelayFree(BuildUse(cas->Comparand())); // Internals may not collide with target setInternalRegsDelayFree = true; diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 522d2e5d0e6eb9..3257b3c04b216b 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -439,9 +439,9 @@ int LinearScan::BuildNode(GenTree* tree) // Comparand is preferenced to RAX. // The remaining two operands can be in any reg other than RAX. - BuildUse(tree->AsCmpXchg()->gtOpLocation, availableIntRegs & ~RBM_RAX); - BuildUse(tree->AsCmpXchg()->gtOpValue, availableIntRegs & ~RBM_RAX); - BuildUse(tree->AsCmpXchg()->gtOpComparand, RBM_RAX); + BuildUse(tree->AsCmpXchg()->Addr(), availableIntRegs & ~RBM_RAX); + BuildUse(tree->AsCmpXchg()->Data(), availableIntRegs & ~RBM_RAX); + BuildUse(tree->AsCmpXchg()->Comparand(), RBM_RAX); BuildDef(tree, RBM_RAX); } break; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8cc25ba6b68bc1..c14de60a20cdc7 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1135,7 +1135,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) // Spill multireg struct arguments that have Assignments or Calls embedded in them. SetNeedsTemp(&arg); } - else if (!argx->OperIsLocalRead() && !argx->OperIsIndir()) + else if (!argx->OperIsLocalRead() && !argx->OperIsLoad()) { // TODO-CQ: handle HWI/SIMD/COMMA nodes in multi-reg morphing. SetNeedsTemp(&arg); @@ -3302,7 +3302,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // Change our argument, as needed, into a value of the appropriate type. assert((structBaseType != TYP_STRUCT) && (genTypeSize(structBaseType) >= originalSize)); - if (argObj->OperIsIndir()) + if (argObj->OperIsLoad()) { assert(argObj->AsIndir()->Size() == genTypeSize(structBaseType)); argObj->SetOper(GT_IND); @@ -3843,7 +3843,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) } else { - assert(argNode->OperIsIndir()); + assert(argNode->OperIsLoad()); GenTree* baseAddr = argNode->AsIndir()->Addr(); var_types addrType = baseAddr->TypeGet(); @@ -8937,8 +8937,12 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } } + // TODO-Bug: Moving the null check to this indirection should nominally check for interference with + // the other operands in case this is a store. However, doing so unconditionally preserves previous + // behavior and "fixes up" field store importation that places the null check in the wrong location + // (before the 'value' operand is evaluated). MorphAddrContext indMac; - if (tree->OperIsIndir()) // TODO-CQ: add more operators here (e. g. atomics). + if (tree->OperIsIndir() && !tree->OperIsAtomicOp()) { // Communicate to FIELD_ADDR morphing that the parent is an indirection. mac = &indMac; @@ -12878,15 +12882,15 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) break; case GT_CMPXCHG: - tree->AsCmpXchg()->gtOpLocation = fgMorphTree(tree->AsCmpXchg()->gtOpLocation); - tree->AsCmpXchg()->gtOpValue = fgMorphTree(tree->AsCmpXchg()->gtOpValue); - tree->AsCmpXchg()->gtOpComparand = fgMorphTree(tree->AsCmpXchg()->gtOpComparand); + tree->AsCmpXchg()->Addr() = fgMorphTree(tree->AsCmpXchg()->Addr()); + tree->AsCmpXchg()->Data() = fgMorphTree(tree->AsCmpXchg()->Data()); + tree->AsCmpXchg()->Comparand() = fgMorphTree(tree->AsCmpXchg()->Comparand()); tree->gtFlags &= (~GTF_EXCEPT & ~GTF_CALL); - tree->gtFlags |= tree->AsCmpXchg()->gtOpLocation->gtFlags & GTF_ALL_EFFECT; - tree->gtFlags |= tree->AsCmpXchg()->gtOpValue->gtFlags & GTF_ALL_EFFECT; - tree->gtFlags |= tree->AsCmpXchg()->gtOpComparand->gtFlags & GTF_ALL_EFFECT; + tree->gtFlags |= tree->AsCmpXchg()->Addr()->gtFlags & GTF_ALL_EFFECT; + tree->gtFlags |= tree->AsCmpXchg()->Data()->gtFlags & GTF_ALL_EFFECT; + tree->gtFlags |= tree->AsCmpXchg()->Comparand()->gtFlags & GTF_ALL_EFFECT; break; case GT_STORE_DYN_BLK: diff --git a/src/coreclr/jit/sideeffects.cpp b/src/coreclr/jit/sideeffects.cpp index cf0b3210e38535..f54e2153123c42 100644 --- a/src/coreclr/jit/sideeffects.cpp +++ b/src/coreclr/jit/sideeffects.cpp @@ -179,13 +179,9 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node) isWrite = true; } #ifdef FEATURE_HW_INTRINSICS - else if (node->OperIsHWIntrinsic()) + else if (node->OperIsHWIntrinsic() && node->AsHWIntrinsic()->OperIsMemoryStoreOrBarrier()) { - if (node->AsHWIntrinsic()->OperIsMemoryStoreOrBarrier()) - { - // For barriers, we model the behavior after GT_MEMORYBARRIER - isWrite = true; - } + isWrite = true; } #endif // FEATURE_HW_INTRINSICS diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 10b95cf1405398..6da5ad7c2c566f 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11648,9 +11648,9 @@ void Compiler::fgValueNumberTree(GenTree* tree) assert(tree->OperIsImplicitIndir()); // special node with an implicit indirections - GenTree* location = cmpXchg->gtOpLocation; // arg1 - GenTree* value = cmpXchg->gtOpValue; // arg2 - GenTree* comparand = cmpXchg->gtOpComparand; // arg3 + GenTree* location = cmpXchg->Addr(); // arg1 + GenTree* value = cmpXchg->Data(); // arg2 + GenTree* comparand = cmpXchg->Comparand(); // arg3 ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet();