diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index b9fcf5c6e459dd..b2a2b4ab190f4e 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)); } } @@ -7790,8 +7809,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)) + { + assert(currentSpill[type] > 0); + currentSpill[type]--; + } } else if (refPosition->RegOptional() && refPosition->assignedReg() == REG_NA) { @@ -7800,8 +7827,18 @@ 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 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]--; + } } JITDUMP(" Max spill for %s is %d\n", varTypeName(type), maxSpill[type]); } diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 22493f4a479018..259670afd60777 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2007,10 +2007,18 @@ class LinearScan : public RegAllocInterface int BuildAddrUses(GenTree* addr, SingleTypeRegSet candidates = RBM_NONE); RefPosition* BuildDef(GenTree* tree, SingleTypeRegSet dstCandidates = RBM_NONE, int multiRegIdx = 0); - 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); + + // 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); + bool constantConsumerReusesOperandReg(GenTree* consumer); + 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); #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 4571e903b4215f..2eeb27555d01f0 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -469,6 +469,29 @@ void LinearScan::associateRefPosWithInterval(RefPosition* rp) checkConflictingDefUse(rp); rp->lastUse = true; } + + 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 + // 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)) + { + prevRP->lastUse = false; + } + } } RefPosition* prevRP = theReferent->recentRefPosition; @@ -2961,6 +2984,259 @@ 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; + } + } +} + +//------------------------------------------------------------------------ +// 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; +} + +//------------------------------------------------------------------------ +// 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 +// 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. +// +// 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. 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) +{ + 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 (!enregisterLocalVars || tree->IsUnusedValue() || (tree->GetRegNum() != REG_NA)) + { + 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()) + { + 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 + + // 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) || constantConsumerReusesOperandReg(treeConsumer)) + { + return nullptr; + } + } + + GenTree* defConsumer = getConsumingNode(defNode); + + if ((defConsumer == nullptr) || (defConsumer == treeConsumer) || constantConsumerReusesOperandReg(defConsumer)) + { + return nullptr; + } + + if (candidate == nullptr) + { + candidate = interval; + } + } + + return candidate; +} + //------------------------------------------------------------------------ // BuildDef: Build one RefTypeDef RefPosition for the given node at given index // @@ -2996,13 +3272,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); + } + } + } + + if (interval == nullptr) + { + interval = newInterval(type); } - Interval* interval = newInterval(type); if (tree->GetRegNum() != REG_NA) { if (!tree->IsMultiRegNode() || (multiRegIdx == 0)) @@ -3026,6 +3316,7 @@ RefPosition* LinearScan::BuildDef(GenTree* tree, SingleTypeRegSet dstCandidates, assert(dstCandidates != RBM_NONE); } #endif // TARGET_X86 + if (pendingDelayFree) { interval->hasInterferingUses = true; 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..c5dcca3659ade2 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_70182/Runtime_70182.cs @@ -0,0 +1,142 @@ +// 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. +// +// 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 Xunit; + +public class Runtime_70182 +{ + private const float Tolerance = 1e-4f; + + // 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; + + [Fact] + public static void TestEntryPoint() + { + TestVector3Normalize(); + TestVector4Normalize(); + TestQuaternionLerp(); + TestMatrix4x4Decompose(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static Vector3 NormalizeVector3(Vector3 v) => Vector3.Normalize(v); + + private static void TestVector3Normalize() + { + Vector3 actual = NormalizeVector3(s_v3); + + 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); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static Vector4 NormalizeVector4(Vector4 v) => Vector4.Normalize(v); + + private static void TestVector4Normalize() + { + 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 Quaternion LerpQuaternion(Quaternion a, Quaternion b, float t) => Quaternion.Lerp(a, b, t); + + private static void TestQuaternionLerp() + { + 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); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool Decompose(Matrix4x4 matrix, out Vector3 scale, out Quaternion rotation, out Vector3 translation) + { + return Matrix4x4.Decompose(matrix, out scale, out rotation, out translation); + } + + private static void TestMatrix4x4Decompose() + { + // 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); + } +} 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 @@ +