From e030f5602330a800393514860a3237f1069d91da Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 10 Oct 2023 17:31:00 +0300 Subject: [PATCH 1/7] Use "GenTreeIndir" for atomic nodes --- src/coreclr/jit/compiler.h | 3 ++ src/coreclr/jit/fgprofile.cpp | 2 +- src/coreclr/jit/gentree.cpp | 32 +++++++++++++++++++ src/coreclr/jit/gentree.h | 51 +++++++++++++++---------------- src/coreclr/jit/importercalls.cpp | 24 ++++----------- 5 files changed, 67 insertions(+), 45 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b5c08a34c30115..0df8b878733a45 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, 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..4a31fdd9a9d8fb 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -8344,6 +8344,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 // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 3605f3bcd33262..32234de5c73931 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -5617,31 +5617,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 +7123,7 @@ struct GenTreeIndir : public GenTreeOp GenTree*& Data() { - assert(OperIs(GT_STOREIND) || OperIsStoreBlk()); + assert(OperIs(GT_STOREIND) || OperIsStoreBlk() || OperIsAtomicOp()); return gtOp2; } @@ -7442,6 +7417,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/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) From 224736dd2544746be522955febe11229250e99f2 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 10 Oct 2023 17:35:36 +0300 Subject: [PATCH 2/7] gtOpLocation -> Addr() --- src/coreclr/jit/codegenarm64.cpp | 2 +- src/coreclr/jit/codegenriscv64.cpp | 2 +- src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/gentree.cpp | 18 +++++++++--------- src/coreclr/jit/lsraarm64.cpp | 2 +- src/coreclr/jit/lsrariscv64.cpp | 2 +- src/coreclr/jit/lsraxarch.cpp | 2 +- src/coreclr/jit/morph.cpp | 4 ++-- src/coreclr/jit/valuenum.cpp | 2 +- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 092a031f270480..94fe8fef98986c 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3930,7 +3930,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) { assert(treeNode->OperIs(GT_CMPXCHG)); - GenTree* addr = treeNode->gtOpLocation; // arg1 + GenTree* addr = treeNode->Addr(); // arg1 GenTree* data = treeNode->gtOpValue; // arg2 GenTree* comparand = treeNode->gtOpComparand; // arg3 diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 4a64ebb374a19f..0d4722a24a5ebd 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -2670,7 +2670,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) { assert(treeNode->OperIs(GT_CMPXCHG)); - GenTree* locOp = treeNode->gtOpLocation; + GenTree* locOp = treeNode->Addr(); GenTree* valOp = treeNode->gtOpValue; GenTree* comparandOp = treeNode->gtOpComparand; diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 16ffe5a8d77117..0039bbbdea252b 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4396,7 +4396,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* tree) var_types targetType = tree->TypeGet(); regNumber targetReg = tree->GetRegNum(); - GenTree* location = tree->gtOpLocation; // arg1 + GenTree* location = tree->Addr(); // arg1 GenTree* value = tree->gtOpValue; // arg2 GenTree* comparand = tree->gtOpComparand; // arg3 diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0df8b878733a45..c61dcb5c5c26fb 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11266,7 +11266,7 @@ class GenTreeVisitor { GenTreeCmpXchg* const cmpXchg = node->AsCmpXchg(); - result = WalkTree(&cmpXchg->gtOpLocation, cmpXchg); + result = WalkTree(&cmpXchg->Addr(), cmpXchg); if (result == fgWalkResult::WALK_ABORT) { return result; diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index a786b56edc29dc..ecd858e6d15560 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4656,7 +4656,7 @@ 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; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 4a31fdd9a9d8fb..3af2c21aa1f6d9 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2994,7 +2994,7 @@ 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) && + return Compare(op1->AsCmpXchg()->Addr(), op2->AsCmpXchg()->Addr()) && Compare(op1->AsCmpXchg()->gtOpValue, op2->AsCmpXchg()->gtOpValue) && Compare(op1->AsCmpXchg()->gtOpComparand, op2->AsCmpXchg()->gtOpComparand); @@ -3546,7 +3546,7 @@ unsigned Compiler::gtHashValue(GenTree* tree) break; case GT_CMPXCHG: - hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpLocation)); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Addr())); hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpValue)); hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpComparand)); break; @@ -6204,8 +6204,8 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) case GT_CMPXCHG: - level = gtSetEvalOrder(tree->AsCmpXchg()->gtOpLocation); - costSz = tree->AsCmpXchg()->gtOpLocation->GetCostSz(); + level = gtSetEvalOrder(tree->AsCmpXchg()->Addr()); + costSz = tree->AsCmpXchg()->Addr()->GetCostSz(); lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->gtOpValue); if (level < lvl2) @@ -6522,9 +6522,9 @@ 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) @@ -9365,7 +9365,7 @@ GenTree* Compiler::gtCloneExpr( case GT_CMPXCHG: copy = new (this, GT_CMPXCHG) GenTreeCmpXchg(tree->TypeGet(), - gtCloneExpr(tree->AsCmpXchg()->gtOpLocation, addFlags, deepVarNum, deepVarVal), + gtCloneExpr(tree->AsCmpXchg()->Addr(), addFlags, deepVarNum, deepVarVal), gtCloneExpr(tree->AsCmpXchg()->gtOpValue, addFlags, deepVarNum, deepVarVal), gtCloneExpr(tree->AsCmpXchg()->gtOpComparand, addFlags, deepVarNum, deepVarVal)); break; @@ -10009,7 +10009,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; @@ -12661,7 +12661,7 @@ void Compiler::gtDispTree(GenTree* tree, if (!topOnly) { - gtDispChild(tree->AsCmpXchg()->gtOpLocation, indentStack, IIArc, nullptr, topOnly); + gtDispChild(tree->AsCmpXchg()->Addr(), indentStack, IIArc, nullptr, topOnly); gtDispChild(tree->AsCmpXchg()->gtOpValue, indentStack, IIArc, nullptr, topOnly); gtDispChild(tree->AsCmpXchg()->gtOpComparand, indentStack, IIArcBottom, nullptr, topOnly); } diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index d3bba13f8ea65d..316f415642f67e 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -975,7 +975,7 @@ 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); setDelayFree(valueUse); diff --git a/src/coreclr/jit/lsrariscv64.cpp b/src/coreclr/jit/lsrariscv64.cpp index 117a46f71fad25..5f577966fe86d2 100644 --- a/src/coreclr/jit/lsrariscv64.cpp +++ b/src/coreclr/jit/lsrariscv64.cpp @@ -375,7 +375,7 @@ int LinearScan::BuildNode(GenTree* tree) 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->Addr())); setDelayFree(BuildUse(cas->gtOpValue)); setDelayFree(BuildUse(cas->gtOpComparand)); diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 522d2e5d0e6eb9..f28f9ba777b068 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -439,7 +439,7 @@ 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()->Addr(), availableIntRegs & ~RBM_RAX); BuildUse(tree->AsCmpXchg()->gtOpValue, availableIntRegs & ~RBM_RAX); BuildUse(tree->AsCmpXchg()->gtOpComparand, RBM_RAX); BuildDef(tree, RBM_RAX); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8cc25ba6b68bc1..2eed9bfd51e7df 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12878,13 +12878,13 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) break; case GT_CMPXCHG: - tree->AsCmpXchg()->gtOpLocation = fgMorphTree(tree->AsCmpXchg()->gtOpLocation); + tree->AsCmpXchg()->Addr() = fgMorphTree(tree->AsCmpXchg()->Addr()); tree->AsCmpXchg()->gtOpValue = fgMorphTree(tree->AsCmpXchg()->gtOpValue); tree->AsCmpXchg()->gtOpComparand = fgMorphTree(tree->AsCmpXchg()->gtOpComparand); tree->gtFlags &= (~GTF_EXCEPT & ~GTF_CALL); - tree->gtFlags |= tree->AsCmpXchg()->gtOpLocation->gtFlags & GTF_ALL_EFFECT; + tree->gtFlags |= tree->AsCmpXchg()->Addr()->gtFlags & GTF_ALL_EFFECT; tree->gtFlags |= tree->AsCmpXchg()->gtOpValue->gtFlags & GTF_ALL_EFFECT; tree->gtFlags |= tree->AsCmpXchg()->gtOpComparand->gtFlags & GTF_ALL_EFFECT; break; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 10b95cf1405398..510bff346a3031 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11648,7 +11648,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) assert(tree->OperIsImplicitIndir()); // special node with an implicit indirections - GenTree* location = cmpXchg->gtOpLocation; // arg1 + GenTree* location = cmpXchg->Addr(); // arg1 GenTree* value = cmpXchg->gtOpValue; // arg2 GenTree* comparand = cmpXchg->gtOpComparand; // arg3 From d124902042f90f362bf8d0f1d8156ea8835de6fe Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 10 Oct 2023 17:36:17 +0300 Subject: [PATCH 3/7] gtOpValue -> Data() --- src/coreclr/jit/codegenarm64.cpp | 2 +- src/coreclr/jit/codegenriscv64.cpp | 2 +- src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/gentree.cpp | 18 +++++++++--------- src/coreclr/jit/lsraarm64.cpp | 2 +- src/coreclr/jit/lsrariscv64.cpp | 2 +- src/coreclr/jit/lsraxarch.cpp | 2 +- src/coreclr/jit/morph.cpp | 4 ++-- src/coreclr/jit/valuenum.cpp | 2 +- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 94fe8fef98986c..8ae42309436d34 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3931,7 +3931,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) assert(treeNode->OperIs(GT_CMPXCHG)); GenTree* addr = treeNode->Addr(); // arg1 - GenTree* data = treeNode->gtOpValue; // arg2 + GenTree* data = treeNode->Data(); // arg2 GenTree* comparand = treeNode->gtOpComparand; // arg3 regNumber targetReg = treeNode->GetRegNum(); diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 0d4722a24a5ebd..04966c258d4f02 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -2671,7 +2671,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) assert(treeNode->OperIs(GT_CMPXCHG)); GenTree* locOp = treeNode->Addr(); - GenTree* valOp = treeNode->gtOpValue; + GenTree* valOp = treeNode->Data(); GenTree* comparandOp = treeNode->gtOpComparand; regNumber target = treeNode->GetRegNum(); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 0039bbbdea252b..62ca7c174e562d 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4397,7 +4397,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* tree) regNumber targetReg = tree->GetRegNum(); GenTree* location = tree->Addr(); // arg1 - GenTree* value = tree->gtOpValue; // arg2 + GenTree* value = tree->Data(); // arg2 GenTree* comparand = tree->gtOpComparand; // arg3 assert(location->GetRegNum() != REG_NA && location->GetRegNum() != REG_RAX); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index c61dcb5c5c26fb..4a3a286cb646fb 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11271,7 +11271,7 @@ class GenTreeVisitor { return result; } - result = WalkTree(&cmpXchg->gtOpValue, cmpXchg); + result = WalkTree(&cmpXchg->Data(), cmpXchg); if (result == fgWalkResult::WALK_ABORT) { return result; diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index ecd858e6d15560..e5a0905130f5e4 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4660,7 +4660,7 @@ void GenTree::VisitOperands(TVisitor visitor) { return; } - if (visitor(cmpXchg->gtOpValue) == VisitResult::Abort) + if (visitor(cmpXchg->Data()) == VisitResult::Abort) { return; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3af2c21aa1f6d9..9888f4afdc3df1 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2995,7 +2995,7 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) case GT_CMPXCHG: return Compare(op1->AsCmpXchg()->Addr(), op2->AsCmpXchg()->Addr()) && - Compare(op1->AsCmpXchg()->gtOpValue, op2->AsCmpXchg()->gtOpValue) && + Compare(op1->AsCmpXchg()->Data(), op2->AsCmpXchg()->Data()) && Compare(op1->AsCmpXchg()->gtOpComparand, op2->AsCmpXchg()->gtOpComparand); case GT_STORE_DYN_BLK: @@ -3547,7 +3547,7 @@ unsigned Compiler::gtHashValue(GenTree* tree) case GT_CMPXCHG: hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Addr())); - hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpValue)); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Data())); hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpComparand)); break; @@ -6207,12 +6207,12 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) level = gtSetEvalOrder(tree->AsCmpXchg()->Addr()); costSz = tree->AsCmpXchg()->Addr()->GetCostSz(); - lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->gtOpValue); + lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->Data()); if (level < lvl2) { level = lvl2; } - costSz += tree->AsCmpXchg()->gtOpValue->GetCostSz(); + costSz += tree->AsCmpXchg()->Data()->GetCostSz(); lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->gtOpComparand); if (level < lvl2) @@ -6527,9 +6527,9 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) *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) @@ -9366,7 +9366,7 @@ GenTree* Compiler::gtCloneExpr( copy = new (this, GT_CMPXCHG) GenTreeCmpXchg(tree->TypeGet(), gtCloneExpr(tree->AsCmpXchg()->Addr(), addFlags, deepVarNum, deepVarVal), - gtCloneExpr(tree->AsCmpXchg()->gtOpValue, addFlags, deepVarNum, deepVarVal), + gtCloneExpr(tree->AsCmpXchg()->Data(), addFlags, deepVarNum, deepVarVal), gtCloneExpr(tree->AsCmpXchg()->gtOpComparand, addFlags, deepVarNum, deepVarVal)); break; @@ -10058,7 +10058,7 @@ 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: @@ -12662,7 +12662,7 @@ void Compiler::gtDispTree(GenTree* tree, if (!topOnly) { gtDispChild(tree->AsCmpXchg()->Addr(), indentStack, IIArc, nullptr, topOnly); - gtDispChild(tree->AsCmpXchg()->gtOpValue, indentStack, IIArc, nullptr, topOnly); + gtDispChild(tree->AsCmpXchg()->Data(), indentStack, IIArc, nullptr, topOnly); gtDispChild(tree->AsCmpXchg()->gtOpComparand, indentStack, IIArcBottom, nullptr, topOnly); } break; diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 316f415642f67e..1a9f000afc5f01 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -977,7 +977,7 @@ int LinearScan::BuildNode(GenTree* tree) 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()) { diff --git a/src/coreclr/jit/lsrariscv64.cpp b/src/coreclr/jit/lsrariscv64.cpp index 5f577966fe86d2..46174e8948ce0e 100644 --- a/src/coreclr/jit/lsrariscv64.cpp +++ b/src/coreclr/jit/lsrariscv64.cpp @@ -376,7 +376,7 @@ int LinearScan::BuildNode(GenTree* tree) buildInternalIntRegisterDefForNode(tree); // temp reg for store conditional error // Extend lifetimes of argument regs because they may be reused during retries setDelayFree(BuildUse(cas->Addr())); - setDelayFree(BuildUse(cas->gtOpValue)); + setDelayFree(BuildUse(cas->Data())); setDelayFree(BuildUse(cas->gtOpComparand)); // Internals may not collide with target diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index f28f9ba777b068..853cdc07bfda7e 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -440,7 +440,7 @@ 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()->Addr(), availableIntRegs & ~RBM_RAX); - BuildUse(tree->AsCmpXchg()->gtOpValue, availableIntRegs & ~RBM_RAX); + BuildUse(tree->AsCmpXchg()->Data(), availableIntRegs & ~RBM_RAX); BuildUse(tree->AsCmpXchg()->gtOpComparand, RBM_RAX); BuildDef(tree, RBM_RAX); } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 2eed9bfd51e7df..f102e00eb8628c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12879,13 +12879,13 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) case GT_CMPXCHG: tree->AsCmpXchg()->Addr() = fgMorphTree(tree->AsCmpXchg()->Addr()); - tree->AsCmpXchg()->gtOpValue = fgMorphTree(tree->AsCmpXchg()->gtOpValue); + tree->AsCmpXchg()->Data() = fgMorphTree(tree->AsCmpXchg()->Data()); tree->AsCmpXchg()->gtOpComparand = fgMorphTree(tree->AsCmpXchg()->gtOpComparand); tree->gtFlags &= (~GTF_EXCEPT & ~GTF_CALL); tree->gtFlags |= tree->AsCmpXchg()->Addr()->gtFlags & GTF_ALL_EFFECT; - tree->gtFlags |= tree->AsCmpXchg()->gtOpValue->gtFlags & GTF_ALL_EFFECT; + tree->gtFlags |= tree->AsCmpXchg()->Data()->gtFlags & GTF_ALL_EFFECT; tree->gtFlags |= tree->AsCmpXchg()->gtOpComparand->gtFlags & GTF_ALL_EFFECT; break; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 510bff346a3031..a80ffb516cf1eb 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11649,7 +11649,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) assert(tree->OperIsImplicitIndir()); // special node with an implicit indirections GenTree* location = cmpXchg->Addr(); // arg1 - GenTree* value = cmpXchg->gtOpValue; // arg2 + GenTree* value = cmpXchg->Data(); // arg2 GenTree* comparand = cmpXchg->gtOpComparand; // arg3 ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet(); From 60503826572b7aa5cbd8688e0d50d88abe73e3ee Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 10 Oct 2023 17:37:17 +0300 Subject: [PATCH 4/7] gtOpComparand -> Comparand() --- src/coreclr/jit/codegenarm64.cpp | 2 +- src/coreclr/jit/codegenriscv64.cpp | 2 +- src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/gentree.cpp | 18 +++++++++--------- src/coreclr/jit/lower.cpp | 2 +- src/coreclr/jit/lsraarm64.cpp | 6 +++--- src/coreclr/jit/lsrariscv64.cpp | 4 ++-- src/coreclr/jit/lsraxarch.cpp | 2 +- src/coreclr/jit/morph.cpp | 4 ++-- src/coreclr/jit/valuenum.cpp | 2 +- 12 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 8ae42309436d34..ffde31b3650018 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3932,7 +3932,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) GenTree* addr = treeNode->Addr(); // arg1 GenTree* data = treeNode->Data(); // arg2 - GenTree* comparand = treeNode->gtOpComparand; // arg3 + 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 04966c258d4f02..81c90fe0937985 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -2672,7 +2672,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) GenTree* locOp = treeNode->Addr(); GenTree* valOp = treeNode->Data(); - GenTree* comparandOp = treeNode->gtOpComparand; + 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 62ca7c174e562d..095e323a7e874a 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4398,7 +4398,7 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* tree) GenTree* location = tree->Addr(); // arg1 GenTree* value = tree->Data(); // arg2 - GenTree* comparand = tree->gtOpComparand; // arg3 + 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 4a3a286cb646fb..43cd60212c7dd9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11276,7 +11276,7 @@ class GenTreeVisitor { 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 e5a0905130f5e4..5f150b20b0d4aa 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4664,7 +4664,7 @@ void GenTree::VisitOperands(TVisitor visitor) { return; } - visitor(cmpXchg->gtOpComparand); + visitor(cmpXchg->Comparand()); return; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9888f4afdc3df1..f8525b76ca914c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2996,7 +2996,7 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) case GT_CMPXCHG: return Compare(op1->AsCmpXchg()->Addr(), op2->AsCmpXchg()->Addr()) && Compare(op1->AsCmpXchg()->Data(), op2->AsCmpXchg()->Data()) && - Compare(op1->AsCmpXchg()->gtOpComparand, op2->AsCmpXchg()->gtOpComparand); + Compare(op1->AsCmpXchg()->Comparand(), op2->AsCmpXchg()->Comparand()); case GT_STORE_DYN_BLK: return Compare(op1->AsStoreDynBlk()->Addr(), op2->AsStoreDynBlk()->Addr()) && @@ -3548,7 +3548,7 @@ unsigned Compiler::gtHashValue(GenTree* tree) case GT_CMPXCHG: hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Addr())); hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Data())); - hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->gtOpComparand)); + hash = genTreeHashAdd(hash, gtHashValue(tree->AsCmpXchg()->Comparand())); break; case GT_STORE_DYN_BLK: @@ -6214,12 +6214,12 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) } costSz += tree->AsCmpXchg()->Data()->GetCostSz(); - lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->gtOpComparand); + lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->Comparand()); if (level < lvl2) { level = lvl2; } - costSz += tree->AsCmpXchg()->gtOpComparand->GetCostSz(); + costSz += tree->AsCmpXchg()->Comparand()->GetCostSz(); costEx = MAX_COST; // Seriously, what could be more expensive than lock cmpxchg? costSz += 5; // size of lock cmpxchg [reg+C], reg @@ -6532,9 +6532,9 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) *pUse = &cmpXchg->Data(); return true; } - if (operand == cmpXchg->gtOpComparand) + if (operand == cmpXchg->Comparand()) { - *pUse = &cmpXchg->gtOpComparand; + *pUse = &cmpXchg->Comparand(); return true; } return false; @@ -9367,7 +9367,7 @@ GenTree* Compiler::gtCloneExpr( GenTreeCmpXchg(tree->TypeGet(), gtCloneExpr(tree->AsCmpXchg()->Addr(), addFlags, deepVarNum, deepVarVal), gtCloneExpr(tree->AsCmpXchg()->Data(), addFlags, deepVarNum, deepVarVal), - gtCloneExpr(tree->AsCmpXchg()->gtOpComparand, addFlags, deepVarNum, deepVarVal)); + gtCloneExpr(tree->AsCmpXchg()->Comparand(), addFlags, deepVarNum, deepVarVal)); break; case GT_STORE_DYN_BLK: @@ -10062,7 +10062,7 @@ void GenTreeUseEdgeIterator::AdvanceCmpXchg() m_state = 1; break; case 1: - m_edge = &m_node->AsCmpXchg()->gtOpComparand; + m_edge = &m_node->AsCmpXchg()->Comparand(); m_advance = &GenTreeUseEdgeIterator::Terminate; break; default: @@ -12663,7 +12663,7 @@ void Compiler::gtDispTree(GenTree* tree, { gtDispChild(tree->AsCmpXchg()->Addr(), indentStack, IIArc, nullptr, topOnly); gtDispChild(tree->AsCmpXchg()->Data(), indentStack, IIArc, nullptr, topOnly); - gtDispChild(tree->AsCmpXchg()->gtOpComparand, indentStack, IIArcBottom, nullptr, topOnly); + gtDispChild(tree->AsCmpXchg()->Comparand(), indentStack, IIArcBottom, nullptr, topOnly); } break; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index bb31df9240a57a..644290e2ead7dc 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: diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 1a9f000afc5f01..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)) @@ -979,9 +979,9 @@ int LinearScan::BuildNode(GenTree* tree) setDelayFree(locationUse); 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 46174e8948ce0e..a9c357aca8ac09 100644 --- a/src/coreclr/jit/lsrariscv64.cpp +++ b/src/coreclr/jit/lsrariscv64.cpp @@ -369,7 +369,7 @@ 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); @@ -377,7 +377,7 @@ int LinearScan::BuildNode(GenTree* tree) // Extend lifetimes of argument regs because they may be reused during retries setDelayFree(BuildUse(cas->Addr())); setDelayFree(BuildUse(cas->Data())); - setDelayFree(BuildUse(cas->gtOpComparand)); + 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 853cdc07bfda7e..3257b3c04b216b 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -441,7 +441,7 @@ int LinearScan::BuildNode(GenTree* tree) // The remaining two operands can be in any reg other than RAX. BuildUse(tree->AsCmpXchg()->Addr(), availableIntRegs & ~RBM_RAX); BuildUse(tree->AsCmpXchg()->Data(), availableIntRegs & ~RBM_RAX); - BuildUse(tree->AsCmpXchg()->gtOpComparand, 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 f102e00eb8628c..f35ab1146968ab 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12880,13 +12880,13 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) case GT_CMPXCHG: tree->AsCmpXchg()->Addr() = fgMorphTree(tree->AsCmpXchg()->Addr()); tree->AsCmpXchg()->Data() = fgMorphTree(tree->AsCmpXchg()->Data()); - tree->AsCmpXchg()->gtOpComparand = fgMorphTree(tree->AsCmpXchg()->gtOpComparand); + tree->AsCmpXchg()->Comparand() = fgMorphTree(tree->AsCmpXchg()->Comparand()); tree->gtFlags &= (~GTF_EXCEPT & ~GTF_CALL); tree->gtFlags |= tree->AsCmpXchg()->Addr()->gtFlags & GTF_ALL_EFFECT; tree->gtFlags |= tree->AsCmpXchg()->Data()->gtFlags & GTF_ALL_EFFECT; - tree->gtFlags |= tree->AsCmpXchg()->gtOpComparand->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/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index a80ffb516cf1eb..0733733f21810a 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11650,7 +11650,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) GenTree* location = cmpXchg->Addr(); // arg1 GenTree* value = cmpXchg->Data(); // arg2 - GenTree* comparand = cmpXchg->gtOpComparand; // arg3 + GenTree* comparand = cmpXchg->Comparand(); // arg3 ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet(); From eeaa22011bcee2289c15487bfecee9441fdc1d87 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 10 Oct 2023 17:46:33 +0300 Subject: [PATCH 5/7] Fix up formatting --- src/coreclr/jit/codegenarm64.cpp | 4 ++-- src/coreclr/jit/codegenxarch.cpp | 4 ++-- src/coreclr/jit/morph.cpp | 4 ++-- src/coreclr/jit/valuenum.cpp | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index ffde31b3650018..1bacf5ffd61abe 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3930,8 +3930,8 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* treeNode) { assert(treeNode->OperIs(GT_CMPXCHG)); - GenTree* addr = treeNode->Addr(); // arg1 - GenTree* data = treeNode->Data(); // arg2 + GenTree* addr = treeNode->Addr(); // arg1 + GenTree* data = treeNode->Data(); // arg2 GenTree* comparand = treeNode->Comparand(); // arg3 regNumber targetReg = treeNode->GetRegNum(); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 095e323a7e874a..a73d96ba10d72d 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4396,8 +4396,8 @@ void CodeGen::genCodeForCmpXchg(GenTreeCmpXchg* tree) var_types targetType = tree->TypeGet(); regNumber targetReg = tree->GetRegNum(); - GenTree* location = tree->Addr(); // arg1 - GenTree* value = tree->Data(); // arg2 + GenTree* location = tree->Addr(); // arg1 + GenTree* value = tree->Data(); // arg2 GenTree* comparand = tree->Comparand(); // arg3 assert(location->GetRegNum() != REG_NA && location->GetRegNum() != REG_RAX); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f35ab1146968ab..649c26086b532b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12878,8 +12878,8 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) break; case GT_CMPXCHG: - tree->AsCmpXchg()->Addr() = fgMorphTree(tree->AsCmpXchg()->Addr()); - tree->AsCmpXchg()->Data() = fgMorphTree(tree->AsCmpXchg()->Data()); + 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); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 0733733f21810a..6da5ad7c2c566f 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11648,8 +11648,8 @@ void Compiler::fgValueNumberTree(GenTree* tree) assert(tree->OperIsImplicitIndir()); // special node with an implicit indirections - GenTree* location = cmpXchg->Addr(); // arg1 - GenTree* value = cmpXchg->Data(); // arg2 + GenTree* location = cmpXchg->Addr(); // arg1 + GenTree* value = cmpXchg->Data(); // arg2 GenTree* comparand = cmpXchg->Comparand(); // arg3 ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet(); From 5a6658085e0029609db6e1bd6dc70b382be50301 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 10 Oct 2023 19:17:50 +0300 Subject: [PATCH 6/7] Make atomics indirections --- src/coreclr/jit/earlyprop.cpp | 2 +- src/coreclr/jit/gentree.cpp | 4 ++-- src/coreclr/jit/gentree.h | 21 +++++++++++++++++++-- src/coreclr/jit/gtlist.h | 17 ++++++++--------- src/coreclr/jit/gtstructs.h | 8 ++++---- src/coreclr/jit/lower.cpp | 4 ++-- src/coreclr/jit/morph.cpp | 12 ++++++++---- src/coreclr/jit/sideeffects.cpp | 8 ++------ 8 files changed, 46 insertions(+), 30 deletions(-) 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/gentree.cpp b/src/coreclr/jit/gentree.cpp index f8525b76ca914c..4b50350e8a73ce 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -6999,7 +6999,7 @@ ExceptionSetFlags GenTree::OperExceptions(Compiler* comp) #endif // FEATURE_HW_INTRINSICS default: - assert(!OperMayOverflow() && !OperIsIndirOrArrMetaData()); + assert(!OperMayOverflow() && (!OperIsIndirOrArrMetaData() || OperIsAtomicZeroDiffQuirk())); return ExceptionSetFlags::None; } } @@ -17521,7 +17521,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 32234de5c73931..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; 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/lower.cpp b/src/coreclr/jit/lower.cpp index 644290e2ead7dc..de70ef47f3eebf 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -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/morph.cpp b/src/coreclr/jit/morph.cpp index 649c26086b532b..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; 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 From 783e017bebf0608aa46b77f9c6d3bd2ebcc02fe6 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Wed, 11 Oct 2023 14:12:34 +0300 Subject: [PATCH 7/7] Massage code for MSVC --- src/coreclr/jit/gentree.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 4b50350e8a73ce..1d78ccbf852f3e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -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()->Addr()); - costSz = tree->AsCmpXchg()->Addr()->GetCostSz(); - - lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->Data()); + GenTree* value = tree->AsCmpXchg()->Data(); + lvl2 = gtSetEvalOrder(value); if (level < lvl2) { level = lvl2; } - costSz += tree->AsCmpXchg()->Data()->GetCostSz(); + costSz += value->GetCostSz(); - lvl2 = gtSetEvalOrder(tree->AsCmpXchg()->Comparand()); + GenTree* comparand = tree->AsCmpXchg()->Comparand(); + lvl2 = gtSetEvalOrder(comparand); if (level < lvl2) { level = lvl2; } - costSz += tree->AsCmpXchg()->Comparand()->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());