From 15ca6dc58489c38b5752e7830448e174f10821c9 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 08:46:31 -0700 Subject: [PATCH 01/13] JIT: Reuse enregistered FP/SIMD/mask constants across non-last uses LSRA previously could only reuse an already-enregistered constant when the earlier occurrence was at its last use, forcing redundant rematerialization (e.g. repeated `xorps`) of identical CNS_DBL/CNS_VEC/CNS_MSK values. At build time, coalesce overlapping identical FP/SIMD/mask constants (still pending in the defList) into a single interval, adding later occurrences as redefinitions. At allocation time, elide a redundant redef via SetReuseRegVal when the register already holds the value. Clear the prior use's lastUse so the coalesced interval remains one continuous range, and never restore a constant previousInterval whose register was borrowed and overwritten (it must be rematerialized instead). These constants have no GC liveness considerations, so coalescing is safe. Fixes #70182 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsra.cpp | 19 +++ src/coreclr/jit/lsra.h | 6 + src/coreclr/jit/lsrabuild.cpp | 150 +++++++++++++++++- .../JitBlue/Runtime_70182/Runtime_70182.cs | 133 ++++++++++++++++ .../JIT/Regression/Regression_o_1.csproj | 1 + 5 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index b9fcf5c6e459dd..2e111672d03ec6 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -4151,6 +4151,14 @@ bool LinearScan::canRestorePreviousInterval(RegRecord* regRec, Interval* assigne (regRec->previousInterval != nullptr && regRec->previousInterval != assignedInterval && regRec->previousInterval->assignedReg == regRec && regRec->previousInterval->getNextRefPosition() != nullptr); + // A constant interval that had its register borrowed by another definition no longer holds its + // value in that register, so it must not be restored (the value would need to be rematerialized). + // Restoring it would leave the register marked as holding a constant it no longer contains. + if (retVal && regRec->previousInterval->isConstant) + { + retVal = false; + } + #ifdef TARGET_ARM if (retVal && regRec->previousInterval->registerType == TYP_DOUBLE) { @@ -6308,6 +6316,17 @@ void LinearScan::allocateRegisters() currentRefPosition.reload = true; } } + else if (RefTypeIsDef(refType) && currentInterval->isConstant && + (currentRefPosition.treeNode != nullptr) && + isRegConstant(assignedRegister, currentInterval->registerType)) + { + // This is a redefinition of a constant interval that is already + // materialized in this register (an identical constant was coalesced + // into this interval at build time). Reuse the value in place rather + // than re-materializing it. + assert(m_compiler->opts.OptimizationEnabled()); + currentRefPosition.treeNode->SetReuseRegVal(); + } INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, currentInterval, assignedRegister)); } } diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 22493f4a479018..b5b9643b8814a1 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2007,6 +2007,12 @@ class LinearScan : public RegAllocInterface int BuildAddrUses(GenTree* addr, SingleTypeRegSet candidates = RBM_NONE); RefPosition* BuildDef(GenTree* tree, SingleTypeRegSet dstCandidates = RBM_NONE, int multiRegIdx = 0); + + // Constant reuse: coalesce an overlapping, identical floating-point/SIMD/mask constant + // into a single interval so the allocator can keep the value in one register. + Interval* getConstantIntervalForReuse(GenTree* tree); + static bool areSameConstantNodes(GenTree* tree1, GenTree* tree2); + void BuildDefs(GenTree* tree, int dstCount, SingleTypeRegSet dstCandidates = RBM_NONE); int BuildCallArgUses(GenTreeCall* call); void BuildCallDefs(GenTree* tree, int dstCount, regMaskTP dstCandidates); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 4571e903b4215f..0a236c7d817631 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -468,6 +468,20 @@ void LinearScan::associateRefPosWithInterval(RefPosition* rp) { checkConflictingDefUse(rp); rp->lastUse = true; + + if (theInterval->isConstant) + { + // This may be a coalesced constant interval, where several identical + // floating-point/SIMD/mask constants share a single interval. In that case an + // earlier use is no longer the last use, so clear its lastUse flag (mirroring + // the localVar handling above) to keep the interval a single continuous live + // range rather than freeing and re-using its register mid-range. + RefPosition* const prevRP = theInterval->recentRefPosition; + if ((prevRP != nullptr) && RefTypeIsUse(prevRP->refType) && (prevRP->bbNum == rp->bbNum)) + { + prevRP->lastUse = false; + } + } } } @@ -2961,6 +2975,136 @@ void setTgtPref(Interval* interval, RefPosition* tgtPrefUse) } #endif // !TARGET_ARM +//------------------------------------------------------------------------ +// areSameConstantNodes: Determine whether two floating-point, SIMD, or mask +// constant nodes represent an identical value that can +// share a single register. +// +// Arguments: +// tree1 - The first constant node +// tree2 - The second constant node +// +// Return Value: +// True iff both nodes are the same kind of FP/SIMD/mask constant, have the +// same type, and hold a bitwise-identical value. +// +// Notes: +// Only GT_CNS_DBL, GT_CNS_VEC, and GT_CNS_MSK are considered. These constants +// have no GC liveness considerations, so they can be freely coalesced. +// +/* static */ bool LinearScan::areSameConstantNodes(GenTree* tree1, GenTree* tree2) +{ + if (tree1->OperGet() != tree2->OperGet()) + { + return false; + } + + if (tree1->TypeGet() != tree2->TypeGet()) + { + return false; + } + + switch (tree1->OperGet()) + { + case GT_CNS_DBL: + { + // For floating point constants, the values must be identical, not simply compare + // equal. So we compare the bits. + return tree1->AsDblCon()->isBitwiseEqual(tree2->AsDblCon()); + } + +#if defined(FEATURE_SIMD) + case GT_CNS_VEC: + { + return GenTreeVecCon::Equals(tree1->AsVecCon(), tree2->AsVecCon()); + } +#endif // FEATURE_SIMD + +#if defined(FEATURE_MASKED_HW_INTRINSICS) + case GT_CNS_MSK: + { + return GenTreeMskCon::Equals(tree1->AsMskCon(), tree2->AsMskCon()); + } +#endif // FEATURE_MASKED_HW_INTRINSICS + + default: + { + return false; + } + } +} + +//------------------------------------------------------------------------ +// getConstantIntervalForReuse: If an identical floating-point/SIMD/mask constant +// is still pending in the defList, return its interval +// so the new definition can be coalesced into it. +// +// Arguments: +// tree - The constant node currently being defined +// +// Return Value: +// The existing constant Interval to reuse, or nullptr if none is available. +// +// Notes: +// A matching def still present in the defList means the earlier constant has +// not yet been consumed and therefore overlaps this definition. Coalescing the +// two into a single interval lets the allocator keep the value in one register +// and elide the redundant re-materialization. +// +// Only applied when optimizing (enregisterLocalVars). Constant nodes are +// single-use in LIR and the defList is per-block, so any match is guaranteed to +// be within the current block. +// +Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) +{ + if (!enregisterLocalVars) + { + return nullptr; + } + + if (!tree->OperIs(GT_CNS_DBL) && !tree->OperIs(GT_CNS_VEC) && !tree->OperIs(GT_CNS_MSK)) + { + return nullptr; + } + + // Only coalesce a plain, single-register definition that feeds a parent use. + if (tree->IsMultiRegNode() || tree->IsUnusedValue() || (tree->GetRegNum() != REG_NA)) + { + return nullptr; + } + + for (RefInfoListNode *listNode = defList.Begin(), *end = defList.End(); listNode != end; + listNode = listNode->Next()) + { + GenTree* defNode = listNode->treeNode; + + if (!areSameConstantNodes(tree, defNode)) + { + continue; + } + + Interval* interval = listNode->ref->getInterval(); + + if (!interval->isConstant) + { + continue; + } + +#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE + // Values needing a partial callee-save may have their upper bits saved/restored + // around calls, so they can't be blindly reused. + if (Compiler::varTypeNeedsPartialCalleeSave(interval->registerType)) + { + continue; + } +#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE + + return interval; + } + + return nullptr; +} + //------------------------------------------------------------------------ // BuildDef: Build one RefTypeDef RefPosition for the given node at given index // @@ -3002,7 +3146,11 @@ RefPosition* LinearScan::BuildDef(GenTree* tree, SingleTypeRegSet dstCandidates, needToKillFloatRegs = true; } - Interval* interval = newInterval(type); + Interval* interval = getConstantIntervalForReuse(tree); + if (interval == nullptr) + { + interval = newInterval(type); + } if (tree->GetRegNum() != REG_NA) { if (!tree->IsMultiRegNode() || (multiRegIdx == 0)) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs new file mode 100644 index 00000000000000..7a28697ac82432 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs @@ -0,0 +1,133 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Regression test for https://github.com/dotnet/runtime/issues/70182 +// +// LSRA can coalesce identical floating-point, SIMD, and mask constants that are +// simultaneously live into a single interval and reuse the already-materialized +// register instead of re-emitting the constant. These tests exercise patterns +// where several identical constants overlap (so they are not folded away by CSE) +// and are consumed in distinct registers/operations, then validate that the +// numeric results are still correct. + +namespace Runtime_70182; + +using System; +using System.Numerics; +using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics; +using Xunit; + +public class Runtime_70182 +{ + [Fact] + public static void TestEntryPoint() + { + TestDoubleConstantReuse(); + TestVectorConstantReuse(); + TestMatrix4x4CreateShadow(); + TestMatrix4x4CreateReflection(); + TestQuaternionNormalizeLerp(); + } + + // Several uses of the same double constant that stay live at once. + [MethodImpl(MethodImplOptions.NoInlining)] + private static double DoubleWork(double x) + { + // 3.5 is used four times; keeping multiple copies live at the same time + // forces the allocator through the coalesce/reuse path rather than a + // trivial single-def/single-use interval. + double a = x * 3.5; + double b = x + 3.5; + double c = x - 3.5; + double d = x / 3.5; + return (a + b) * (c + d) + 3.5; + } + + private static void TestDoubleConstantReuse() + { + double result = DoubleWork(7.0); + + double a = 7.0 * 3.5; + double b = 7.0 + 3.5; + double c = 7.0 - 3.5; + double d = 7.0 / 3.5; + double expected = (a + b) * (c + d) + 3.5; + + Assert.Equal(expected, result); + } + + // Several uses of the same vector constant that stay live at once. + [MethodImpl(MethodImplOptions.NoInlining)] + private static Vector128 VectorWork(Vector128 x) + { + Vector128 k = Vector128.Create(2.0f, 3.0f, 4.0f, 5.0f); + + // Each expression re-references the identical constant while earlier + // references are still live. + Vector128 a = x * k; + Vector128 b = x + k; + Vector128 c = a - k; + Vector128 d = b + k; + return (a + b) + (c + d); + } + + private static void TestVectorConstantReuse() + { + Vector128 x = Vector128.Create(1.0f, 2.0f, 3.0f, 4.0f); + Vector128 result = VectorWork(x); + + Vector128 k = Vector128.Create(2.0f, 3.0f, 4.0f, 5.0f); + Vector128 a = x * k; + Vector128 b = x + k; + Vector128 c = a - k; + Vector128 d = b + k; + Vector128 expected = (a + b) + (c + d); + + Assert.Equal(expected, result); + } + + // Matrix4x4.CreateShadow materializes several identical zero vectors; this is + // one of the shapes that regressed without register-tracking fixes. + [MethodImpl(MethodImplOptions.NoInlining)] + private static Matrix4x4 CreateShadow(Vector3 lightDirection, Plane plane) + { + return Matrix4x4.CreateShadow(lightDirection, plane); + } + + private static void TestMatrix4x4CreateShadow() + { + var light = new Vector3(1.0f, -2.0f, 3.0f); + var plane = new Plane(0.0f, 1.0f, 0.0f, 0.0f); + + Assert.Equal(Matrix4x4.CreateShadow(light, plane), CreateShadow(light, plane)); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static Matrix4x4 CreateReflection(Plane plane) + { + return Matrix4x4.CreateReflection(plane); + } + + private static void TestMatrix4x4CreateReflection() + { + var plane = Plane.Normalize(new Plane(1.0f, 2.0f, 3.0f, 4.0f)); + + Assert.Equal(Matrix4x4.CreateReflection(plane), CreateReflection(plane)); + } + + // Quaternion.Lerp with normalization produces overlapping zero/mask constants. + [MethodImpl(MethodImplOptions.NoInlining)] + private static Quaternion Lerp(Quaternion a, Quaternion b, float t) + { + return Quaternion.Lerp(a, b, t); + } + + private static void TestQuaternionNormalizeLerp() + { + var a = Quaternion.Normalize(new Quaternion(1.0f, 2.0f, 3.0f, 4.0f)); + var b = Quaternion.Normalize(new Quaternion(-4.0f, 3.0f, -2.0f, 1.0f)); + + Assert.Equal(Quaternion.Lerp(a, b, 0.25f), Lerp(a, b, 0.25f)); + } +} diff --git a/src/tests/JIT/Regression/Regression_o_1.csproj b/src/tests/JIT/Regression/Regression_o_1.csproj index f079fcb880c2a7..0bab337e15c6c5 100644 --- a/src/tests/JIT/Regression/Regression_o_1.csproj +++ b/src/tests/JIT/Regression/Regression_o_1.csproj @@ -141,6 +141,7 @@ + From 1bf76601d861d8c4d885cd0b8683082210daa98b Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 10:38:34 -0700 Subject: [PATCH 02/13] Fix formatting and build --- src/coreclr/jit/lsra.h | 8 ++++---- src/coreclr/jit/lsrabuild.cpp | 18 ++++++++++++++++-- .../JitBlue/Runtime_70182/Runtime_70182.cs | 1 - 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index b5b9643b8814a1..c0b0b74ba18d5a 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2013,10 +2013,10 @@ class LinearScan : public RegAllocInterface Interval* getConstantIntervalForReuse(GenTree* tree); static bool areSameConstantNodes(GenTree* tree1, GenTree* tree2); - void BuildDefs(GenTree* tree, int dstCount, SingleTypeRegSet dstCandidates = RBM_NONE); - int BuildCallArgUses(GenTreeCall* call); - void BuildCallDefs(GenTree* tree, int dstCount, regMaskTP dstCandidates); - void BuildKills(GenTree* tree, regMaskTP killMask); + void BuildDefs(GenTree* tree, int dstCount, SingleTypeRegSet dstCandidates = RBM_NONE); + int BuildCallArgUses(GenTreeCall* call); + void BuildCallDefs(GenTree* tree, int dstCount, regMaskTP dstCandidates); + void BuildKills(GenTree* tree, regMaskTP killMask); #if defined(TARGET_ARMARCH) || defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64) void BuildDefWithKills(GenTree* tree, SingleTypeRegSet dstCandidates, regMaskTP killMask); #else diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 0a236c7d817631..10f92ca1d699ae 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3062,10 +3062,24 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) return nullptr; } - if (!tree->OperIs(GT_CNS_DBL) && !tree->OperIs(GT_CNS_VEC) && !tree->OperIs(GT_CNS_MSK)) + bool canHandle = false; + + if (tree->OperIs(GT_CNS_DBL)) { - return nullptr; + canHandle = true; } +#if defined(FEATURE_HW_INTRINSICS) + else if (tree->OperIs(GT_CNS_VEC)) + { + canHandle = true; + } +#endif +#if defined(FEATURE_MASKED_HW_INTRINSICS) + else if (tree->OperIs(GT_CNS_MSK)) + { + canHandle = true; + } +#endif // Only coalesce a plain, single-register definition that feeds a parent use. if (tree->IsMultiRegNode() || tree->IsUnusedValue() || (tree->GetRegNum() != REG_NA)) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs index 7a28697ac82432..ff2f00e95cd34c 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs @@ -12,7 +12,6 @@ namespace Runtime_70182; -using System; using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.Intrinsics; From ef8308fb5f874f26296209eadd9bb34f168c1a89 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 10:47:01 -0700 Subject: [PATCH 03/13] JIT: Clear prior use lastUse when coalescing a later constant def Address review feedback: getConstantIntervalForReuse can coalesce a later constant definition (RefTypeDef) into an interval after an earlier occurrence has already been consumed (RefTypeUse). The prior lastUse-clearing only ran in the RefTypeUse branch, leaving that earlier use marked as lastUse and allowing the coalesced interval to be freed mid-range. Move the clearing so it runs for every reference added to a constant interval. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsrabuild.cpp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 10f92ca1d699ae..117d9379368634 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -468,19 +468,25 @@ void LinearScan::associateRefPosWithInterval(RefPosition* rp) { checkConflictingDefUse(rp); rp->lastUse = true; + } - if (theInterval->isConstant) + if (theInterval->isConstant) + { + // This may be a coalesced constant interval, where several identical + // floating-point/SIMD/mask constants share a single interval. Once another + // reference (a later use, or a later definition coalesced into the interval via + // getConstantIntervalForReuse) is added, any earlier use is no longer the last + // use, so clear its lastUse flag (mirroring the localVar handling above) to keep + // the interval a single continuous live range rather than freeing and re-using its + // register mid-range. + // + // This is checked for every reference (not just uses) because a RefTypeDef can + // follow a RefTypeUse when a third occurrence is coalesced after an earlier one has + // already been consumed. + RefPosition* const prevRP = theInterval->recentRefPosition; + if ((prevRP != nullptr) && RefTypeIsUse(prevRP->refType) && (prevRP->bbNum == rp->bbNum)) { - // This may be a coalesced constant interval, where several identical - // floating-point/SIMD/mask constants share a single interval. In that case an - // earlier use is no longer the last use, so clear its lastUse flag (mirroring - // the localVar handling above) to keep the interval a single continuous live - // range rather than freeing and re-using its register mid-range. - RefPosition* const prevRP = theInterval->recentRefPosition; - if ((prevRP != nullptr) && RefTypeIsUse(prevRP->refType) && (prevRP->bbNum == rp->bbNum)) - { - prevRP->lastUse = false; - } + prevRP->lastUse = false; } } } From a670f4a6325e45537ef40661cf7db19e587a7b17 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 10:57:08 -0700 Subject: [PATCH 04/13] Ensure canHandle is used --- src/coreclr/jit/lsrabuild.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 117d9379368634..7733c1768a4deb 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3087,6 +3087,11 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) } #endif + if (!canHandle) + { + return nullptr; + } + // Only coalesce a plain, single-register definition that feeds a parent use. if (tree->IsMultiRegNode() || tree->IsUnusedValue() || (tree->GetRegNum() != REG_NA)) { From ac9f45432575086468c21cf2aace8ab21934214a Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 12:47:12 -0700 Subject: [PATCH 05/13] JIT: Don't coalesce constants that share a consuming node Fixes an ARM32 LSRA assert (lsra.cpp:6121) in cases such as System.Numerics.Vector3:get_Zero, where FEATURE_SIMD is off and 'return default(Vector3)' lowers to RETURN(FIELD_LIST(CNS_DBL 0, CNS_DBL 0, CNS_DBL 0)). Each FIELD_LIST operand is used at the same LSRA location (the RETURN) but requires a distinct fixed ABI return register. Coalescing the identical constants into a single interval forced that interval to occupy multiple registers at once. getConstantIntervalForReuse now skips coalescing when the new constant and any already-coalesced pending definition share a consuming node (resolved through contained users via getConsumingNode), since that is exactly when two uses land at the same location. Constants feeding different consumers (the intended reuse case) are unaffected. Also wires up the previously-unused canHandle early-out. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsra.h | 1 + src/coreclr/jit/lsrabuild.cpp | 79 ++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index c0b0b74ba18d5a..e485b5c5de6494 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2011,6 +2011,7 @@ class LinearScan : public RegAllocInterface // Constant reuse: coalesce an overlapping, identical floating-point/SIMD/mask constant // into a single interval so the allocator can keep the value in one register. Interval* getConstantIntervalForReuse(GenTree* tree); + GenTree* getConsumingNode(GenTree* node); static bool areSameConstantNodes(GenTree* tree1, GenTree* tree2); void BuildDefs(GenTree* tree, int dstCount, SingleTypeRegSet dstCandidates = RBM_NONE); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 7733c1768a4deb..7e65b5346f9b93 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3040,6 +3040,42 @@ void setTgtPref(Interval* interval, RefPosition* tgtPrefUse) } } +//------------------------------------------------------------------------ +// getConsumingNode: Find the non-contained node that ultimately consumes the +// value produced by 'node' within the current block. +// +// Arguments: +// node - The node whose consumer is sought +// +// Return Value: +// The first non-contained user of 'node', walking through any contained +// intermediate users (e.g. a contained FIELD_LIST), or nullptr if no such +// user exists. +// +// Notes: +// The returned node identifies the LSRA location at which 'node' is used, +// since uses are built when the consuming (non-contained) node is processed. +// This is used to detect when two constants would be used at the same location. +// +GenTree* LinearScan::getConsumingNode(GenTree* node) +{ + LIR::Range& blockRange = LIR::AsRange(m_compiler->compCurBB); + + LIR::Use use; + while (blockRange.TryGetUse(node, &use)) + { + GenTree* user = use.User(); + + if ((user == nullptr) || !user->isContained()) + { + return user; + } + + node = user; + } + + return nullptr; +} //------------------------------------------------------------------------ // getConstantIntervalForReuse: If an identical floating-point/SIMD/mask constant // is still pending in the defList, return its interval @@ -3057,6 +3093,10 @@ void setTgtPref(Interval* interval, RefPosition* tgtPrefUse) // two into a single interval lets the allocator keep the value in one register // and elide the redundant re-materialization. // +// Constants that are consumed by the same node are not coalesced, since that +// would require the shared interval to be in multiple registers at a single +// location (see below). +// // Only applied when optimizing (enregisterLocalVars). Constant nodes are // single-use in LIR and the defList is per-block, so any match is guaranteed to // be within the current block. @@ -3098,6 +3138,12 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) return nullptr; } + // The consumer of 'tree' is computed lazily on the first matching candidate, since most + // definitions will not find one and we want to avoid the range walk otherwise. + GenTree* treeConsumer = nullptr; + bool gotTreeConsumer = false; + Interval* candidate = nullptr; + for (RefInfoListNode *listNode = defList.Begin(), *end = defList.End(); listNode != end; listNode = listNode->Next()) { @@ -3124,10 +3170,39 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) } #endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE - return interval; + // Two constants that are consumed by the same node must not be coalesced: doing so + // would force the single resulting interval to satisfy multiple uses at the same + // location, each of which may require a distinct fixed register (for example the + // operands of a FIELD_LIST feeding a multi-register return or call argument). A single + // interval cannot occupy several registers at once. Because an interval may already + // have earlier definitions coalesced into it, every matching pending definition is + // checked, and any conflict (or an undeterminable consumer) disables coalescing for + // this constant entirely so it is materialized independently. + if (!gotTreeConsumer) + { + treeConsumer = getConsumingNode(tree); + gotTreeConsumer = true; + + if (treeConsumer == nullptr) + { + return nullptr; + } + } + + GenTree* defConsumer = getConsumingNode(defNode); + + if ((defConsumer == nullptr) || (defConsumer == treeConsumer)) + { + return nullptr; + } + + if (candidate == nullptr) + { + candidate = interval; + } } - return nullptr; + return candidate; } //------------------------------------------------------------------------ From 6f31cfdbf3305026c737df97e46ee6cf935279ed Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 13:19:07 -0700 Subject: [PATCH 06/13] JIT: Only reuse enregistered constants when optimizing The constant-reuse coalescing has no effect in MinOpts (enregisterLocalVars disabled), and running the defList scan there added measurable throughput cost (up to +0.34%). Gate the whole helper on OptimizationEnabled to match the existing isMatchingConstant reuse path and avoid that overhead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsrabuild.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 7e65b5346f9b93..11451ded6d6605 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3097,13 +3097,12 @@ GenTree* LinearScan::getConsumingNode(GenTree* node) // would require the shared interval to be in multiple registers at a single // location (see below). // -// Only applied when optimizing (enregisterLocalVars). Constant nodes are -// single-use in LIR and the defList is per-block, so any match is guaranteed to -// be within the current block. +// Only applied when optimizing. Constant nodes are single-use in LIR and the +// defList is per-block, so any match is guaranteed to be within the current block. // Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) { - if (!enregisterLocalVars) + if (!enregisterLocalVars || !m_compiler->opts.OptimizationEnabled()) { return nullptr; } From 1877202d9a9741a28aef7c95232c6f45f586df95 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 14:13:39 -0700 Subject: [PATCH 07/13] JIT: Reshape Runtime_70182 test to exercise the constant-reuse path The previous test defined its vector constant once in a local (a single GT_CNS_VEC) and consumed each scalar literal immediately, so it never drove getConstantIntervalForReuse. Reshape it around System.Numerics shapes that actually keep identical FP/SIMD constants live across distinct consumers (Vector3/Vector4.Normalize, Quaternion.Lerp, Matrix4x4.Decompose), running each at full opts behind NoInlining wrappers with runtime inputs and validating against an independent scalar reference within tolerance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JitBlue/Runtime_70182/Runtime_70182.cs | 186 +++++++++--------- 1 file changed, 98 insertions(+), 88 deletions(-) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs index ff2f00e95cd34c..c5dcca3659ade2 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs @@ -5,128 +5,138 @@ // // LSRA can coalesce identical floating-point, SIMD, and mask constants that are // simultaneously live into a single interval and reuse the already-materialized -// register instead of re-emitting the constant. These tests exercise patterns -// where several identical constants overlap (so they are not folded away by CSE) -// and are consumed in distinct registers/operations, then validate that the -// numeric results are still correct. +// register instead of re-emitting the constant. +// +// The shapes below are the ones that actually drive that path: they come from +// System.Numerics vector math (Vector3/Vector4.Normalize, Quaternion.Lerp, +// Matrix4x4.Decompose), where the SIMD zero/one constants used by the reciprocal, +// square-root, and sign/compare sequences stay live across several distinct +// consumers. On hardware with AVX-512 masking those constants are register +// resident (rather than contained memory operands), so the coalescing/redefinition +// logic runs and, prior to the fix, could mis-assign registers or trip an LSRA +// assert. Each test drives the shape at full opts and validates the result against +// an independent scalar reference, so a bad register reuse (which produces garbage +// rather than a small rounding difference) is caught. namespace Runtime_70182; using System.Numerics; using System.Runtime.CompilerServices; -using System.Runtime.Intrinsics; using Xunit; public class Runtime_70182 { - [Fact] - public static void TestEntryPoint() - { - TestDoubleConstantReuse(); - TestVectorConstantReuse(); - TestMatrix4x4CreateShadow(); - TestMatrix4x4CreateReflection(); - TestQuaternionNormalizeLerp(); - } + private const float Tolerance = 1e-4f; - // Several uses of the same double constant that stay live at once. - [MethodImpl(MethodImplOptions.NoInlining)] - private static double DoubleWork(double x) - { - // 3.5 is used four times; keeping multiple copies live at the same time - // forces the allocator through the coalesce/reuse path rather than a - // trivial single-def/single-use interval. - double a = x * 3.5; - double b = x + 3.5; - double c = x - 3.5; - double d = x / 3.5; - return (a + b) * (c + d) + 3.5; - } + // Kept in non-readonly statics so the JIT cannot constant-fold the operations + // away and must actually generate the vector-math code under test. + private static Vector3 s_v3 = new Vector3(1.5f, -2.5f, 3.5f); + private static Vector4 s_v4 = new Vector4(1.5f, -2.5f, 3.5f, -4.5f); + private static Quaternion s_qa = new Quaternion(1.0f, 2.0f, 3.0f, 4.0f); + private static Quaternion s_qb = new Quaternion(-4.0f, 3.0f, -2.0f, 1.0f); + private static float s_amount = 0.25f; - private static void TestDoubleConstantReuse() + [Fact] + public static void TestEntryPoint() { - double result = DoubleWork(7.0); - - double a = 7.0 * 3.5; - double b = 7.0 + 3.5; - double c = 7.0 - 3.5; - double d = 7.0 / 3.5; - double expected = (a + b) * (c + d) + 3.5; - - Assert.Equal(expected, result); + TestVector3Normalize(); + TestVector4Normalize(); + TestQuaternionLerp(); + TestMatrix4x4Decompose(); } - // Several uses of the same vector constant that stay live at once. [MethodImpl(MethodImplOptions.NoInlining)] - private static Vector128 VectorWork(Vector128 x) - { - Vector128 k = Vector128.Create(2.0f, 3.0f, 4.0f, 5.0f); - - // Each expression re-references the identical constant while earlier - // references are still live. - Vector128 a = x * k; - Vector128 b = x + k; - Vector128 c = a - k; - Vector128 d = b + k; - return (a + b) + (c + d); - } + private static Vector3 NormalizeVector3(Vector3 v) => Vector3.Normalize(v); - private static void TestVectorConstantReuse() + private static void TestVector3Normalize() { - Vector128 x = Vector128.Create(1.0f, 2.0f, 3.0f, 4.0f); - Vector128 result = VectorWork(x); + Vector3 actual = NormalizeVector3(s_v3); - Vector128 k = Vector128.Create(2.0f, 3.0f, 4.0f, 5.0f); - Vector128 a = x * k; - Vector128 b = x + k; - Vector128 c = a - k; - Vector128 d = b + k; - Vector128 expected = (a + b) + (c + d); - - Assert.Equal(expected, result); + double len = System.Math.Sqrt((double)s_v3.X * s_v3.X + (double)s_v3.Y * s_v3.Y + (double)s_v3.Z * s_v3.Z); + Assert.Equal((float)(s_v3.X / len), actual.X, Tolerance); + Assert.Equal((float)(s_v3.Y / len), actual.Y, Tolerance); + Assert.Equal((float)(s_v3.Z / len), actual.Z, Tolerance); } - // Matrix4x4.CreateShadow materializes several identical zero vectors; this is - // one of the shapes that regressed without register-tracking fixes. [MethodImpl(MethodImplOptions.NoInlining)] - private static Matrix4x4 CreateShadow(Vector3 lightDirection, Plane plane) - { - return Matrix4x4.CreateShadow(lightDirection, plane); - } + private static Vector4 NormalizeVector4(Vector4 v) => Vector4.Normalize(v); - private static void TestMatrix4x4CreateShadow() + private static void TestVector4Normalize() { - var light = new Vector3(1.0f, -2.0f, 3.0f); - var plane = new Plane(0.0f, 1.0f, 0.0f, 0.0f); - - Assert.Equal(Matrix4x4.CreateShadow(light, plane), CreateShadow(light, plane)); + Vector4 actual = NormalizeVector4(s_v4); + + double len = System.Math.Sqrt((double)s_v4.X * s_v4.X + (double)s_v4.Y * s_v4.Y + + (double)s_v4.Z * s_v4.Z + (double)s_v4.W * s_v4.W); + Assert.Equal((float)(s_v4.X / len), actual.X, Tolerance); + Assert.Equal((float)(s_v4.Y / len), actual.Y, Tolerance); + Assert.Equal((float)(s_v4.Z / len), actual.Z, Tolerance); + Assert.Equal((float)(s_v4.W / len), actual.W, Tolerance); } [MethodImpl(MethodImplOptions.NoInlining)] - private static Matrix4x4 CreateReflection(Plane plane) - { - return Matrix4x4.CreateReflection(plane); - } + private static Quaternion LerpQuaternion(Quaternion a, Quaternion b, float t) => Quaternion.Lerp(a, b, t); - private static void TestMatrix4x4CreateReflection() + private static void TestQuaternionLerp() { - var plane = Plane.Normalize(new Plane(1.0f, 2.0f, 3.0f, 4.0f)); - - Assert.Equal(Matrix4x4.CreateReflection(plane), CreateReflection(plane)); + Quaternion a = Quaternion.Normalize(s_qa); + Quaternion b = Quaternion.Normalize(s_qb); + Quaternion actual = LerpQuaternion(a, b, s_amount); + + // Reference lerp: blend towards the closer orientation, then normalize. + float dot = a.X * b.X + a.Y * b.Y + a.Z * b.Z + a.W * b.W; + float t = s_amount; + float s = 1.0f - t; + float sign = dot >= 0.0f ? t : -t; + + float rx = s * a.X + sign * b.X; + float ry = s * a.Y + sign * b.Y; + float rz = s * a.Z + sign * b.Z; + float rw = s * a.W + sign * b.W; + double rlen = System.Math.Sqrt((double)rx * rx + (double)ry * ry + (double)rz * rz + (double)rw * rw); + + Assert.Equal((float)(rx / rlen), actual.X, Tolerance); + Assert.Equal((float)(ry / rlen), actual.Y, Tolerance); + Assert.Equal((float)(rz / rlen), actual.Z, Tolerance); + Assert.Equal((float)(rw / rlen), actual.W, Tolerance); + + // The lerp result is a unit quaternion regardless of the register the + // constants land in; a bad reuse would break this invariant. + double actualLen = System.Math.Sqrt((double)actual.X * actual.X + (double)actual.Y * actual.Y + + (double)actual.Z * actual.Z + (double)actual.W * actual.W); + Assert.Equal(1.0f, (float)actualLen, Tolerance); } - // Quaternion.Lerp with normalization produces overlapping zero/mask constants. [MethodImpl(MethodImplOptions.NoInlining)] - private static Quaternion Lerp(Quaternion a, Quaternion b, float t) + private static bool Decompose(Matrix4x4 matrix, out Vector3 scale, out Quaternion rotation, out Vector3 translation) { - return Quaternion.Lerp(a, b, t); + return Matrix4x4.Decompose(matrix, out scale, out rotation, out translation); } - private static void TestQuaternionNormalizeLerp() + private static void TestMatrix4x4Decompose() { - var a = Quaternion.Normalize(new Quaternion(1.0f, 2.0f, 3.0f, 4.0f)); - var b = Quaternion.Normalize(new Quaternion(-4.0f, 3.0f, -2.0f, 1.0f)); - - Assert.Equal(Quaternion.Lerp(a, b, 0.25f), Lerp(a, b, 0.25f)); + // Compose a matrix from known scale/rotation/translation, then decompose it. + Vector3 scale = new Vector3(2.0f, 3.0f, 4.0f); + Quaternion rotation = Quaternion.Normalize(new Quaternion(0.3f, 0.5f, 0.1f, 0.8f)); + Vector3 translation = new Vector3(5.0f, -6.0f, 7.0f); + + Matrix4x4 matrix = Matrix4x4.CreateScale(scale) * + Matrix4x4.CreateFromQuaternion(rotation) * + Matrix4x4.CreateTranslation(translation); + + bool success = Decompose(matrix, out Vector3 outScale, out Quaternion outRotation, out Vector3 outTranslation); + + Assert.True(success); + Assert.Equal(scale.X, outScale.X, Tolerance); + Assert.Equal(scale.Y, outScale.Y, Tolerance); + Assert.Equal(scale.Z, outScale.Z, Tolerance); + Assert.Equal(translation.X, outTranslation.X, Tolerance); + Assert.Equal(translation.Y, outTranslation.Y, Tolerance); + Assert.Equal(translation.Z, outTranslation.Z, Tolerance); + + // Rotation may come back negated (q and -q are the same orientation); compare + // via the absolute dot product, which is 1 for equivalent unit quaternions. + float dot = rotation.X * outRotation.X + rotation.Y * outRotation.Y + + rotation.Z * outRotation.Z + rotation.W * outRotation.W; + Assert.Equal(1.0f, System.Math.Abs(dot), Tolerance); } } From 96500f009525c9494d9778916229d1bb979277a2 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 16:48:56 -0700 Subject: [PATCH 08/13] Make the constant reuse early out check be less costly in minopts --- src/coreclr/jit/lsrabuild.cpp | 46 ++++++++++++----------------------- 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 11451ded6d6605..9a0e6a778760f3 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3102,37 +3102,12 @@ GenTree* LinearScan::getConsumingNode(GenTree* node) // Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) { - if (!enregisterLocalVars || !m_compiler->opts.OptimizationEnabled()) - { - return nullptr; - } - - bool canHandle = false; - - if (tree->OperIs(GT_CNS_DBL)) - { - canHandle = true; - } -#if defined(FEATURE_HW_INTRINSICS) - else if (tree->OperIs(GT_CNS_VEC)) - { - canHandle = true; - } -#endif -#if defined(FEATURE_MASKED_HW_INTRINSICS) - else if (tree->OperIs(GT_CNS_MSK)) - { - canHandle = true; - } -#endif - - if (!canHandle) - { - return nullptr; - } + assert(m_compiler->opts.OptimizationEnabled()); + assert(!tree->IsMultiRegNode()); + assert(tree->IsCnsFltOrDbl() || tree->IsCnsVec() || tree->IsCnsMsk()); // Only coalesce a plain, single-register definition that feeds a parent use. - if (tree->IsMultiRegNode() || tree->IsUnusedValue() || (tree->GetRegNum() != REG_NA)) + if (!enregisterLocalVars || tree->IsUnusedValue() || (tree->GetRegNum() != REG_NA)) { return nullptr; } @@ -3239,17 +3214,27 @@ RefPosition* LinearScan::BuildDef(GenTree* tree, SingleTypeRegSet dstCandidates, type = tree->GetRegTypeByIndex(multiRegIdx); } + Interval* interval = nullptr; + if (!varTypeUsesIntReg(type)) { m_compiler->compFloatingPointUsed = true; needToKillFloatRegs = true; + + if (m_compiler->opts.OptimizationEnabled()) + { + if (tree->IsCnsFltOrDbl() || tree->IsCnsVec() || tree->IsCnsMsk()) + { + interval = getConstantIntervalForReuse(tree); + } + } } - Interval* interval = getConstantIntervalForReuse(tree); if (interval == nullptr) { interval = newInterval(type); } + if (tree->GetRegNum() != REG_NA) { if (!tree->IsMultiRegNode() || (multiRegIdx == 0)) @@ -3273,6 +3258,7 @@ RefPosition* LinearScan::BuildDef(GenTree* tree, SingleTypeRegSet dstCandidates, assert(dstCandidates != RBM_NONE); } #endif // TARGET_X86 + if (pendingDelayFree) { interval->hasInterferingUses = true; From a8c6f21680ac2e1bad8f187261b2b0372d4b6bb4 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 17:16:42 -0700 Subject: [PATCH 09/13] JIT: Fix spill-temp accounting for multi-use coalesced constants A coalesced FP/SIMD/mask constant interval can have several uses that read the value directly from a single spill temp. updateMaxSpill was designed for single-use tree temps and decremented the live spill count at every such use, underflowing currentSpill (assert 'currentSpill[type] > 0' at lsra.cpp:7822) on x86 where register pressure causes the constant to be spilled and used from memory more than once. Only release the temp on the last use; earlier uses keep it reserved. No-op for single-use intervals. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsra.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 2e111672d03ec6..2cfb6451945cb8 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -7819,8 +7819,16 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) // memory location. To properly account max spill for typ we // decrement spill count. assert(RefTypeIsUse(refType)); - assert(currentSpill[type] > 0); - currentSpill[type]--; + + // A coalesced constant interval can have several uses that read the value directly + // from a single spill temp. The temp stays live until the final such use, so only + // release it on the last use; earlier uses must leave the temp reserved. For all + // other (single-use) intervals the use is always the last use, so this is a no-op. + if (refPosition->lastUse) + { + assert(currentSpill[type] > 0); + currentSpill[type]--; + } } JITDUMP(" Max spill for %s is %d\n", varTypeName(type), maxSpill[type]); } From d5d623f18d07324c4781c2262c432d695c93a7fa Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 17:16:42 -0700 Subject: [PATCH 10/13] JIT: Skip constant lastUse fixup for integer constants Only floating-point/SIMD/mask constants are ever coalesced (areSameConstantNodes), so integer constant intervals are never multi-ref. Gate the lastUse-clearing on a non-integer register type to avoid the (harmless but pointless) work on every integer constant reference. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsrabuild.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 9a0e6a778760f3..5e04555056f84c 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -470,8 +470,11 @@ void LinearScan::associateRefPosWithInterval(RefPosition* rp) rp->lastUse = true; } - if (theInterval->isConstant) + if (theInterval->isConstant && (regType(theInterval->registerType) != IntRegisterType)) { + // Only floating-point/SIMD/mask constants can be coalesced (see areSameConstantNodes), + // so integer constant intervals are never multi-ref and are skipped here. + // // This may be a coalesced constant interval, where several identical // floating-point/SIMD/mask constants share a single interval. Once another // reference (a later use, or a later definition coalesced into the interval via From 3eab7f3d4d01427b17c3c1c9df29d404e2eeb5ee Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 7 Jul 2026 22:29:48 -0700 Subject: [PATCH 11/13] JIT: Don't coalesce FP/SIMD/mask constants feeding an RMW consumer Coalescing identical enregistered FP/SIMD/mask constants into a single multi-reference interval is unsafe when one of the shared uses is a read-modify-write operand (e.g. an FMA addend or a non-VEX binary op on xarch). The RMW reuses the operand's register for its result, clobbering the constant at the point of use while later references of the coalesced interval still expect the value. Because a coalesced constant is spillable, under register pressure the allocator may store-spill the already-clobbered register and reload garbage for the later uses, producing wrong results (observed under JitStressRegs=0x2/0x3/0x7). Decline coalescing whenever a candidate constant's consumer is such an RMW operation. This is intentionally conservative (it does not distinguish which operand is reused) and can be relaxed once spilled FP/SIMD/mask constants are rematerialized instead of stored and reloaded. Also fix spill-temp accounting in updateMaxSpill: a coalesced constant can be reloaded and immediately re-spilled at the same refposition, which leaves the concurrent spill count unchanged, so the reload must not decrement the count in that case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsra.cpp | 11 ++++++- src/coreclr/jit/lsra.h | 1 + src/coreclr/jit/lsrabuild.cpp | 60 +++++++++++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 2cfb6451945cb8..d8574af835e592 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -7810,7 +7810,16 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) else if (refPosition->reload) { assert(currentSpill[type] > 0); - currentSpill[type]--; + + // A coalesced constant interval can be reloaded for one use and immediately + // re-spilled for a later use at the same refposition. The reload releases the + // spill temp while the re-spill reserves a new one, so the concurrent spill + // count is unchanged. Only a reload without a subsequent spill frees the temp. + // For all other (single-use) intervals reload and spillAfter never coincide. + if (!(interval->isConstant && refPosition->spillAfter)) + { + currentSpill[type]--; + } } else if (refPosition->RegOptional() && refPosition->assignedReg() == REG_NA) { diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index e485b5c5de6494..259670afd60777 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2012,6 +2012,7 @@ class LinearScan : public RegAllocInterface // into a single interval so the allocator can keep the value in one register. Interval* getConstantIntervalForReuse(GenTree* tree); GenTree* getConsumingNode(GenTree* node); + bool constantConsumerReusesOperandReg(GenTree* consumer); static bool areSameConstantNodes(GenTree* tree1, GenTree* tree2); void BuildDefs(GenTree* tree, int dstCount, SingleTypeRegSet dstCandidates = RBM_NONE); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 5e04555056f84c..bfe9c67dc6540e 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3079,6 +3079,61 @@ GenTree* LinearScan::getConsumingNode(GenTree* node) return nullptr; } + +//------------------------------------------------------------------------ +// constantConsumerReusesOperandReg: Determine whether 'consumer' reads-modifies-writes +// one of its register operands, reusing that operand's register for its own result. +// +// Arguments: +// consumer - The non-contained node that consumes a constant value (or nullptr) +// +// Return Value: +// True if 'consumer' may reuse one of its source registers for its destination +// (read-modify-write), or if the consumer could not be determined. +// +// Notes: +// A coalesced (multi-reference) constant that feeds such a consumer is unsafe: when +// the shared register is reused for the consumer's result, it is clobbered at the very +// point of use, while later references of the coalesced interval still expect the +// constant value. Because a coalesced constant is spillable, the allocator may then +// store-spill the (already clobbered) register and reload that garbage for the later +// uses, producing incorrect results under register pressure. +// +// This is intentionally conservative: it declines coalescing whenever any consumer is a +// read-modify-write operation, without distinguishing which operand is reused. The +// restriction can be relaxed once spilled floating-point/SIMD/mask constants are +// rematerialized rather than stored and reloaded. +// +bool LinearScan::constantConsumerReusesOperandReg(GenTree* consumer) +{ + if (consumer == nullptr) + { + // The consumer could not be determined, so conservatively assume the worst. + return true; + } + +#ifdef FEATURE_HW_INTRINSICS + if (consumer->OperIsHWIntrinsic()) + { + // Read-modify-write intrinsics (for example the fused-multiply-add family) reuse one + // of their source registers as the destination. This is the common cross-target case. + return consumer->isRMWHWIntrinsic(m_compiler); + } +#endif // FEATURE_HW_INTRINSICS + +#ifdef TARGET_XARCH + // Without VEX, a binary floating-point operation reuses op1's register as its destination. + // With VEX (the common case) such operations use the non-destructive three-operand form, so + // no register is reused; checking the ISA first lets us skip the operand/oper inspection. + if (!m_compiler->canUseVexEncoding() && consumer->OperIsBinary()) + { + return isRMWRegOper(consumer); + } +#endif // TARGET_XARCH + + return false; +} + //------------------------------------------------------------------------ // getConstantIntervalForReuse: If an identical floating-point/SIMD/mask constant // is still pending in the defList, return its interval @@ -3160,7 +3215,7 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) treeConsumer = getConsumingNode(tree); gotTreeConsumer = true; - if (treeConsumer == nullptr) + if ((treeConsumer == nullptr) || constantConsumerReusesOperandReg(treeConsumer)) { return nullptr; } @@ -3168,7 +3223,8 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) GenTree* defConsumer = getConsumingNode(defNode); - if ((defConsumer == nullptr) || (defConsumer == treeConsumer)) + if ((defConsumer == nullptr) || (defConsumer == treeConsumer) || + constantConsumerReusesOperandReg(defConsumer)) { return nullptr; } From 867b49243f1044b0e6e366466e256f6768f56911 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 8 Jul 2026 00:00:46 -0700 Subject: [PATCH 12/13] Apply formatting patch --- src/coreclr/jit/lsrabuild.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index bfe9c67dc6540e..2eeb27555d01f0 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3223,8 +3223,7 @@ Interval* LinearScan::getConstantIntervalForReuse(GenTree* tree) GenTree* defConsumer = getConsumingNode(defNode); - if ((defConsumer == nullptr) || (defConsumer == treeConsumer) || - constantConsumerReusesOperandReg(defConsumer)) + if ((defConsumer == nullptr) || (defConsumer == treeConsumer) || constantConsumerReusesOperandReg(defConsumer)) { return nullptr; } From ef52d3466ca9e7d9b9bebe52f312fc13a7dd0344 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 8 Jul 2026 00:22:46 -0700 Subject: [PATCH 13/13] JIT: Fix spill-temp accounting for coalesced constants read from memory A constant use that is not assigned a register reads its value directly from the read-only data section, not from a spill temp, so it does not free a spill temp. The RegOptional/memory branch in updateMaxSpill was still decrementing currentSpill for such uses, which could drive the count below zero (asserting 'currentSpill[type] > 0') and, worse, under-count maxSpill so codegen under-allocated spill temps. Skip the decrement for constant intervals: a constant def is only store-spilled when a later use needs the value in a register (a reload), and that temp is released on the reload instead. Also assert-guard the reload branch consistently. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/lsra.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index d8574af835e592..b2a2b4ab190f4e 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -7809,8 +7809,6 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) } else if (refPosition->reload) { - assert(currentSpill[type] > 0); - // A coalesced constant interval can be reloaded for one use and immediately // re-spilled for a later use at the same refposition. The reload releases the // spill temp while the re-spill reserves a new one, so the concurrent spill @@ -7818,6 +7816,7 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) // For all other (single-use) intervals reload and spillAfter never coincide. if (!(interval->isConstant && refPosition->spillAfter)) { + assert(currentSpill[type] > 0); currentSpill[type]--; } } @@ -7829,11 +7828,13 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) // decrement spill count. assert(RefTypeIsUse(refType)); - // A coalesced constant interval can have several uses that read the value directly - // from a single spill temp. The temp stays live until the final such use, so only - // release it on the last use; earlier uses must leave the temp reserved. For all - // other (single-use) intervals the use is always the last use, so this is a no-op. - if (refPosition->lastUse) + // A constant use that is not assigned a register reads its value directly from + // the read-only data section rather than from a spill temp, so there is no spill + // temp to release here. A constant def is only store-spilled when a later use + // needs the value in a register (i.e. that use is a reload), and the temp is + // released on that reload instead. For all other intervals the memory operand is + // the spill temp, which this use frees. + if (!interval->isConstant) { assert(currentSpill[type] > 0); currentSpill[type]--;