From 39a5c790ed67506a8a2ab00e580aa2a363e402aa Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sun, 10 May 2026 11:57:18 -0700 Subject: [PATCH 1/4] Ensure that we consistently copy otherRegs --- src/coreclr/jit/compiler.h | 2 - src/coreclr/jit/gentree.cpp | 80 ++++++++++++++++++++++++------------- src/coreclr/jit/gentree.h | 45 +++++++++++++++++++++ 3 files changed, 98 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 875106b746944b..4d833785ddf8a0 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3994,9 +3994,7 @@ class Compiler void gtDispLeaf(GenTree* tree, IndentStack* indentStack); void gtDispLocal(GenTreeLclVarCommon* tree, IndentStack* indentStack); void gtDispNodeName(GenTree* tree); -#if FEATURE_MULTIREG_RET unsigned gtDispMultiRegCount(GenTree* tree); -#endif void gtDispRegVal(GenTree* tree); void gtDispVN(GenTree* tree); void gtDispCommonEndLine(GenTree* tree); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d91d1961bc2ba4..aca3f2f2f4838a 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -748,16 +748,44 @@ void GenTree::CopyReg(GenTree* from) _gtRegNum = from->_gtRegNum; INDEBUG(gtRegTag = from->gtRegTag;) - // Also copy multi-reg state if this is a call node +#if FEATURE_MULTIREG_RET if (IsCall()) { assert(from->IsCall()); this->AsCall()->CopyOtherRegs(from->AsCall()); + return; + } + +#if !defined(TARGET_64BIT) + if (OperIsMultiRegOp()) + { + this->AsMultiRegOp()->CopyOtherRegs(from->AsMultiRegOp()); + return; } - else if (IsCopyOrReload()) +#endif +#endif // FEATURE_MULTIREG_RET + + if (IsCopyOrReload()) { this->AsCopyOrReload()->CopyOtherRegs(from->AsCopyOrReload()); + return; } + +#if FEATURE_HW_INTRINSICS + if (OperIsHWIntrinsic()) + { + this->AsHWIntrinsic()->CopyOtherRegs(from->AsHWIntrinsic()); + return; + } +#endif + + if (IsMultiRegLclVar()) + { + this->AsLclVar()->CopyOtherRegs(from->AsLclVar()); + return; + } + + assert(!IsMultiRegNode()); } //------------------------------------------------------------------ @@ -857,44 +885,46 @@ bool GenTree::gtHasReg(Compiler* comp) const int GenTree::GetRegisterDstCount(Compiler* compiler) const { assert(!isContained()); - if (!IsMultiRegNode()) - { - return IsValue() ? 1 : 0; - } - else if (IsMultiRegCall()) + +#if FEATURE_MULTIREG_RET + if (IsCall()) { return AsCall()->GetReturnTypeDesc()->GetReturnRegCount(); } - else if (IsCopyOrReload()) - { - return gtGetOp1()->GetRegisterDstCount(compiler); - } + #if !defined(TARGET_64BIT) - else if (OperIsMultiRegOp()) + if (OperIsMultiRegOp()) { assert(OperIs(GT_MUL_LONG)); return 2; } #endif -#ifdef FEATURE_HW_INTRINSICS - else if (OperIsHWIntrinsic()) +#endif // FEATURE_MULTIREG_RET + + if (IsCopyOrReload()) { - assert(TypeIs(TYP_STRUCT)); + return gtGetOp1()->GetRegisterDstCount(compiler); + } - const GenTreeHWIntrinsic* intrinsic = AsHWIntrinsic(); - const NamedIntrinsic intrinsicId = intrinsic->GetHWIntrinsicId(); - assert(HWIntrinsicInfo::IsMultiReg(intrinsicId)); +#ifdef FEATURE_HW_INTRINSICS + if (OperIsHWIntrinsic()) + { + const NamedIntrinsic intrinsicId = AsHWIntrinsic()->GetHWIntrinsicId(); - return HWIntrinsicInfo::GetMultiRegCount(intrinsicId); + if (HWIntrinsicInfo::IsMultiReg(intrinsicId)) + { + return HWIntrinsicInfo::GetMultiRegCount(intrinsicId); + } } #endif // FEATURE_HW_INTRINSICS - if (OperIsScalarLocal()) + if (IsMultiRegLclVar()) { return AsLclVar()->GetFieldCount(compiler); } - assert(!"Unexpected multi-reg node"); - return 0; + + assert(!IsMultiRegNode()); + return IsValue() ? 1 : 0; } //----------------------------------------------------------------------------------- @@ -927,7 +957,7 @@ bool GenTree::IsMultiRegNode() const #endif #endif // FEATURE_MULTIREG_RET - if (OperIs(GT_COPY, GT_RELOAD)) + if (IsCopyOrReload()) { return true; } @@ -13021,7 +13051,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ } } -#if FEATURE_MULTIREG_RET //---------------------------------------------------------------------------------- // gtDispMultiRegCount: determine how many registers to print for a multi-reg node // @@ -13067,7 +13096,6 @@ unsigned Compiler::gtDispMultiRegCount(GenTree* tree) return tree->GetMultiRegCount(this); } } -#endif // FEATURE_MULTIREG_RET //---------------------------------------------------------------------------------- // gtDispRegVal: Print the register(s) defined by the given node @@ -13090,7 +13118,6 @@ void Compiler::gtDispRegVal(GenTree* tree) return; } -#if FEATURE_MULTIREG_RET if (tree->IsMultiRegNode()) { // 0th reg is GetRegNum(), which is already printed above. @@ -13104,7 +13131,6 @@ void Compiler::gtDispRegVal(GenTree* tree) printf(",%s", genIsValidReg(reg) ? compRegVarName(reg) : "NA"); } } -#endif } // We usually/commonly don't expect to print anything longer than this string, diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 85b8693ce996a3..dd2fe23025b9a4 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3954,6 +3954,23 @@ struct GenTreeLclVar : public GenTreeLclVarCommon ClearOtherRegFlags(); } + //---------------------------------------------------------------------------- + // CopyOtherRegs: copy multi-reg state from the given node to this node + // + // Arguments: + // from - Node from which to copy multi-reg state + // + // Return Value: + // None + // + void CopyOtherRegs(GenTreeLclVar* from) + { + for (int i = 0; i < MAX_MULTIREG_COUNT - 1; i++) + { + gtOtherReg[i] = from->gtOtherReg[i]; + } + } + regNumber GetRegNumByIdx(int regIndex) const { assert(regIndex < MAX_MULTIREG_COUNT); @@ -5945,6 +5962,20 @@ struct GenTreeMultiRegOp : public GenTreeOp return TypeIs(TYP_LONG) ? 2 : 1; } + //---------------------------------------------------------------------------- + // CopyOtherRegs: copy multi-reg state from the given node to this node + // + // Arguments: + // from - Node from which to copy multi-reg state + // + // Return Value: + // None + // + void CopyOtherRegs(GenTreeMultiRegOp* from) + { + gtOtherReg = from->gtOtherReg; + } + //--------------------------------------------------------------------------- // GetRegNumByIdx: get i'th register allocated to this struct argument. // @@ -6449,6 +6480,20 @@ struct GenTreeJitIntrinsic : public GenTreeMultiOp } #endif // FEATURE_READYTORUN + //---------------------------------------------------------------------------- + // CopyOtherRegs: copy multi-reg state from the given node to this node + // + // Arguments: + // from - Node from which to copy multi-reg state + // + // Return Value: + // None + // + void CopyOtherRegs(GenTreeJitIntrinsic* from) + { + gtOtherReg = from->gtOtherReg; + } + //----------------------------------------------------------- // GetRegNumByIdx: Get regNumber of i'th position. // From 1753c4a477c8918c5f2fc8d1fec0d1ee477f90b9 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sun, 10 May 2026 12:53:16 -0700 Subject: [PATCH 2/4] Fix an ifdef and ensure we check IsMultiRegCall --- src/coreclr/jit/gentree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index aca3f2f2f4838a..dd5f1284d91773 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -771,7 +771,7 @@ void GenTree::CopyReg(GenTree* from) return; } -#if FEATURE_HW_INTRINSICS +#ifdef FEATURE_HW_INTRINSICS if (OperIsHWIntrinsic()) { this->AsHWIntrinsic()->CopyOtherRegs(from->AsHWIntrinsic()); @@ -887,7 +887,7 @@ int GenTree::GetRegisterDstCount(Compiler* compiler) const assert(!isContained()); #if FEATURE_MULTIREG_RET - if (IsCall()) + if (IsMultiRegCall()) { return AsCall()->GetReturnTypeDesc()->GetReturnRegCount(); } From 9184ec3b2dffef32cbdbfc0ea14ea1f3f84e7f6e Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 11 May 2026 16:19:37 -0700 Subject: [PATCH 3/4] Ensure we consistently copy the spill flags for multireg --- src/coreclr/jit/gentree.cpp | 2 -- src/coreclr/jit/gentree.h | 42 +++++++------------------------------ 2 files changed, 7 insertions(+), 37 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index c483e0f3fe35ff..9d4acc14e7c831 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -11460,8 +11460,6 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree) copy->gtInlineContext = tree->gtInlineContext; - copy->CopyOtherRegFlags(tree); - // We keep track of the number of no return calls, so if we've cloned // one of these, update the tracking. // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index df9c898fae213d..372401df6400a3 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3971,6 +3971,7 @@ struct GenTreeLclVar : public GenTreeLclVarCommon { gtOtherReg[i] = from->gtOtherReg[i]; } + gtSpillFlags = from->gtSpillFlags; } regNumber GetRegNumByIdx(int regIndex) const @@ -4020,21 +4021,6 @@ struct GenTreeLclVar : public GenTreeLclVarCommon gtSpillFlags = 0; } - //------------------------------------------------------------------------- - // CopyOtherRegFlags: copy GTF_* flags associated with gtOtherRegs from - // the given LclVar node. - // - // Arguments: - // fromCall - GenTreeLclVar node from which to copy - // - // Return Value: - // None - // - void CopyOtherRegFlags(GenTreeLclVar* from) - { - this->gtSpillFlags = from->gtSpillFlags; - } - #ifdef DEBUG void ResetLclILoffs() { @@ -5347,8 +5333,9 @@ struct GenTreeCall final : public GenTree #if FEATURE_MULTIREG_RET for (unsigned i = 0; i < MAX_RET_REG_COUNT - 1; ++i) { - this->gtOtherRegs[i] = fromCall->gtOtherRegs[i]; + gtOtherRegs[i] = fromCall->gtOtherRegs[i]; } + gtSpillFlags = fromCall->gtSpillFlags; #endif } @@ -5391,23 +5378,6 @@ struct GenTreeCall final : public GenTree #endif } - //------------------------------------------------------------------------- - // CopyOtherRegFlags: copy GTF_* flags associated with gtOtherRegs from - // the given call node. - // - // Arguments: - // fromCall - GenTreeCall node from which to copy - // - // Return Value: - // None - // - void CopyOtherRegFlags(GenTreeCall* fromCall) - { -#if FEATURE_MULTIREG_RET - this->gtSpillFlags = fromCall->gtSpillFlags; -#endif - } - bool IsUnmanaged() const { return (gtFlags & GTF_CALL_UNMANAGED) != 0; @@ -5975,7 +5945,8 @@ struct GenTreeMultiRegOp : public GenTreeOp // void CopyOtherRegs(GenTreeMultiRegOp* from) { - gtOtherReg = from->gtOtherReg; + gtOtherReg = from->gtOtherReg; + gtSpillFlags = from->gtSpillFlags; } //--------------------------------------------------------------------------- @@ -6493,7 +6464,8 @@ struct GenTreeJitIntrinsic : public GenTreeMultiOp // void CopyOtherRegs(GenTreeJitIntrinsic* from) { - gtOtherReg = from->gtOtherReg; + gtOtherReg = from->gtOtherReg; + gtSpillFlags = from->gtSpillFlags; } //----------------------------------------------------------- From d27ae1ffa963f1b48d69f5cd80d5cef0629d8f0f Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 12 May 2026 20:54:15 -0700 Subject: [PATCH 4/4] Remove CopyReg as we should not have assigned registers during clone or cse --- src/coreclr/jit/gentree.cpp | 59 +++-------------------------- src/coreclr/jit/gentree.h | 74 ------------------------------------- src/coreclr/jit/optcse.cpp | 4 +- 3 files changed, 8 insertions(+), 129 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9d4acc14e7c831..539688edc33ae7 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -737,57 +737,6 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const return compiler->typGetObjLayout(structHnd); } -//----------------------------------------------------------- -// CopyReg: Copy the _gtRegNum/gtRegTag fields. -// -// Arguments: -// from - GenTree node from which to copy -// -void GenTree::CopyReg(GenTree* from) -{ - _gtRegNum = from->_gtRegNum; - INDEBUG(gtRegTag = from->gtRegTag;) - -#if FEATURE_MULTIREG_RET - if (IsCall()) - { - assert(from->IsCall()); - this->AsCall()->CopyOtherRegs(from->AsCall()); - return; - } - -#if !defined(TARGET_64BIT) - if (OperIsMultiRegOp()) - { - this->AsMultiRegOp()->CopyOtherRegs(from->AsMultiRegOp()); - return; - } -#endif -#endif // FEATURE_MULTIREG_RET - - if (IsCopyOrReload()) - { - this->AsCopyOrReload()->CopyOtherRegs(from->AsCopyOrReload()); - return; - } - -#ifdef FEATURE_HW_INTRINSICS - if (OperIsHWIntrinsic()) - { - this->AsHWIntrinsic()->CopyOtherRegs(from->AsHWIntrinsic()); - return; - } -#endif - - if (IsMultiRegLclVar()) - { - this->AsLclVar()->CopyOtherRegs(from->AsLclVar()); - return; - } - - assert(!IsMultiRegNode()); -} - //------------------------------------------------------------------ // gtHasReg: Whether node been assigned a register by LSRA // @@ -11311,7 +11260,10 @@ GenTree* Compiler::gtCloneExpr(GenTree* tree) /* Make sure to copy back fields that may have been initialized */ copy->CopyRawCosts(tree); - copy->CopyReg(tree); + + assert(tree->GetRegNum() == REG_NA); + assert(tree->GetRegTag() == GenTree::GT_REGTAG_NONE); + return copy; } @@ -11501,7 +11453,8 @@ GenTreeCall* Compiler::gtCloneCandidateCall(GenTreeCall* call) result->gtDebugFlags |= (call->gtDebugFlags & ~GTF_DEBUG_NODE_MASK); #endif - result->CopyReg(call); + assert(call->GetRegNum() == REG_NA); + assert(call->GetRegTag() == GenTree::GT_REGTAG_NONE); return result; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 372401df6400a3..73864cdccee92c 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1097,8 +1097,6 @@ struct GenTree INDEBUG(gtRegTag = GT_REGTAG_NONE;) } - // Copy the _gtRegNum/gtRegTag fields - void CopyReg(GenTree* from); bool gtHasReg(Compiler* comp) const; int GetRegisterDstCount(Compiler* compiler) const; @@ -3956,24 +3954,6 @@ struct GenTreeLclVar : public GenTreeLclVarCommon ClearOtherRegFlags(); } - //---------------------------------------------------------------------------- - // CopyOtherRegs: copy multi-reg state from the given node to this node - // - // Arguments: - // from - Node from which to copy multi-reg state - // - // Return Value: - // None - // - void CopyOtherRegs(GenTreeLclVar* from) - { - for (int i = 0; i < MAX_MULTIREG_COUNT - 1; i++) - { - gtOtherReg[i] = from->gtOtherReg[i]; - } - gtSpillFlags = from->gtSpillFlags; - } - regNumber GetRegNumByIdx(int regIndex) const { assert(regIndex < MAX_MULTIREG_COUNT); @@ -5934,21 +5914,6 @@ struct GenTreeMultiRegOp : public GenTreeOp return TypeIs(TYP_LONG) ? 2 : 1; } - //---------------------------------------------------------------------------- - // CopyOtherRegs: copy multi-reg state from the given node to this node - // - // Arguments: - // from - Node from which to copy multi-reg state - // - // Return Value: - // None - // - void CopyOtherRegs(GenTreeMultiRegOp* from) - { - gtOtherReg = from->gtOtherReg; - gtSpillFlags = from->gtSpillFlags; - } - //--------------------------------------------------------------------------- // GetRegNumByIdx: get i'th register allocated to this struct argument. // @@ -6453,21 +6418,6 @@ struct GenTreeJitIntrinsic : public GenTreeMultiOp } #endif // FEATURE_READYTORUN - //---------------------------------------------------------------------------- - // CopyOtherRegs: copy multi-reg state from the given node to this node - // - // Arguments: - // from - Node from which to copy multi-reg state - // - // Return Value: - // None - // - void CopyOtherRegs(GenTreeJitIntrinsic* from) - { - gtOtherReg = from->gtOtherReg; - gtSpillFlags = from->gtSpillFlags; - } - //----------------------------------------------------------- // GetRegNumByIdx: Get regNumber of i'th position. // @@ -8988,30 +8938,6 @@ struct GenTreeCopyOrReload : public GenTreeUnOp } } - //---------------------------------------------------------------------------- - // CopyOtherRegs: copy multi-reg state from the given copy/reload node to this - // node. - // - // Arguments: - // from - GenTree node from which to copy multi-reg state - // - // Return Value: - // None - // - // TODO-ARM: Implement this routine for Arm64 and Arm32 - // TODO-X86: Implement this routine for x86 - void CopyOtherRegs(GenTreeCopyOrReload* from) - { - assert(OperGet() == from->OperGet()); - -#ifdef UNIX_AMD64_ABI - for (unsigned i = 0; i < MAX_MULTIREG_COUNT - 1; ++i) - { - gtOtherRegs[i] = from->gtOtherRegs[i]; - } -#endif - } - unsigned GetRegCount() const { // We need to return the highest index for which we have a valid register. diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 52648649a4cadf..b2baa7fc129c97 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -5234,8 +5234,8 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate) // void CSE_HeuristicCommon::ReplaceCSENode(Statement* stmt, GenTree* exp, GenTree* newNode) { - newNode->CopyReg(exp); // The cse inheirits any reg num property from the original exp node - exp->ClearRegNum(); // The exp node (for a CSE def) no longer has a register requirement + assert(exp->GetRegNum() == REG_NA); + assert(exp->GetRegTag() == GenTree::GT_REGTAG_NONE); // Walk the statement 'stmt' and find the pointer // in the tree is pointing to 'exp'