From 2d7b3f1acb504dc6f16facabf542be1f24017be8 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 19:19:55 +0200 Subject: [PATCH 1/5] JIT: don't propagate promoted SIMD LCL_VAR into FIELD_LIST uses Fixes #128373. Under tiered compilation with tiered PGO on the SysV x64 ABI, the JIT could produce an illegal GT_LCL_VAR of a promoted (non-DNER) SIMD local inside a GT_FIELD_LIST entry of a multi-reg return, hitting an assert in Lowering::CheckNode in Checked builds and segfaulting LSRA's processBlockStartLocations in Release builds. Two phases could introduce the bad IR: * Local copy-assertion propagation in Morph - Global, propagating a `V_dest == V_src` assertion created by MorphCopyBlock onto a later whole-struct LCL_VAR use of V_dest, rewriting it to V_src where V_src is a promoted (non-DNER) SIMD local. * Forward substitution (including the one Physical Promotion runs inline), substituting a STORE_LCL_VAR's source promoted SIMD LCL_VAR directly into a FIELD_LIST entry that no later phase repairs. Both sites now bail out when the source is a promoted, non-DNER SIMD local; restricting to SIMD keeps the guards narrow enough to avoid codegen impact in benchmarks/coreclr_tests SPMI collections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/assertionprop.cpp | 13 ++ src/coreclr/jit/forwardsub.cpp | 20 +++ .../JitBlue/Runtime_128373/Runtime_128373.cs | 118 ++++++++++++++++++ .../Runtime_128373/Runtime_128373.csproj | 13 ++ 4 files changed, 164 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.csproj diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e9af1ed6f1dab3..80aa21d06fffb6 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3489,6 +3489,19 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion, } } + // Don't propagate a promoted SIMD local into a whole-struct GT_LCL_VAR use. + // SIMD locals can be promoted into their lane/element fields (e.g. Quaternion + // -> 4 floats); the resulting "fully" promoted SIMD local has no maintained + // stack home and Lowering::CheckNode rejects a GT_LCL_VAR of it. Unlike + // generic struct copies (which Morph decomposes via MorphCopyBlock at the + // use site), substituting a promoted SIMD local can survive Morph and end + // up as e.g. a GT_FIELD_LIST entry that no later phase repairs. + // + if (tree->OperIs(GT_LCL_VAR) && varTypeIsSIMD(tree) && copyVarDsc->lvPromoted && !copyVarDsc->lvDoNotEnregister) + { + return nullptr; + } + tree->SetLclNum(copyLclNum); // Copy prop and last-use copy elision happens at the same time in morph. diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index e67959aa7c981f..8b1e317043de38 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -791,6 +791,26 @@ bool Compiler::fgForwardSubStatement(Statement* stmt) dstVarDsc->SetIsMultiRegDest(); } + // A GT_LCL_VAR read of a promoted (non-DNER) SIMD local does not + // satisfy the post-lowering invariant in Lowering::CheckNode unless the + // use sits in a context that the lowering phase repairs (e.g. multi-reg-dest + // STORE_LCL_VAR or directly under a GT_RETURN). A GT_FIELD_LIST entry is + // not one of those contexts, so block the substitution. + // + if (fwdSubNode->OperIs(GT_LCL_VAR) && varTypeIsSIMD(fwdSubNode)) + { + LclVarDsc* const fwdSubVarDsc = lvaGetDesc(fwdSubNode->AsLclVar()); + if (fwdSubVarDsc->lvPromoted && !fwdSubVarDsc->lvDoNotEnregister) + { + GenTree* const parentNode = fsv.GetParentNode(); + if ((parentNode != nullptr) && parentNode->OperIs(GT_FIELD_LIST)) + { + JITDUMP(" promoted SIMD lcl var, parent is FIELD_LIST\n"); + return false; + } + } + } + // If a method returns a multi-reg type, only forward sub locals, // and ensure the local and operand have the required markup. // (see eg impFixupStructReturnType). diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.cs b/src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.cs new file mode 100644 index 00000000000000..b403449322c7af --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.cs @@ -0,0 +1,118 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Threading; +using Xunit; +using NumericsMatrix4x4 = System.Numerics.Matrix4x4; +using NumericsQuaternion = System.Numerics.Quaternion; +using NumericsVector3 = System.Numerics.Vector3; + +// Repro for https://github.com/dotnet/runtime/issues/128373. +// +// Under tiered compilation with tiered PGO on the SysV x64 ABI, copy assertion +// propagation and forward substitution could both introduce an illegal +// `LCL_VAR` of a promoted (non-DNER) struct local into a `FIELD_LIST` entry of +// a multi-reg return, hitting a Lowering::CheckNode assert in Checked builds +// and segfaulting LSRA in Release builds. + +public class Runtime_128373 +{ + [Fact] + public static void TestEntryPoint() + { + // Repeatedly invoke ProblematicBody until tiered compilation produces + // the optimized version that used to hit the assert. + for (int i = 0; i < 300; i++) + { + CreateWrappedInputs(i, out var forward, out var up); + WrappedQuaternion q = ProblematicBody(forward, up); + Assert.False(float.IsNaN(q.x)); + Thread.Sleep(16); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void CreateWrappedInputs(int i, out WrappedVector3 forward, out WrappedVector3 up) + { + forward = new WrappedVector3(0.25f + (i * 0.00001f), 1.25f, -0.75f).normalized; + up = new WrappedVector3(0.05f, 1.0f, 0.15f + (i * 0.00002f)).normalized; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WrappedQuaternion ProblematicBody(WrappedVector3 forward, WrappedVector3 upwards) + { + var epsilonSquared = WrappedVector3.kEpsilon * WrappedVector3.kEpsilon; + var forwardVector = WrappedVector3.ToNumerics(forward); + if (forwardVector.LengthSquared() <= epsilonSquared) + return WrappedQuaternion.identity; + + var forwardNormalized = NumericsVector3.Normalize(forwardVector); + var upVector = WrappedVector3.ToNumerics(upwards); + var upNormalized = upVector.LengthSquared() <= epsilonSquared ? NumericsVector3.UnitY : NumericsVector3.Normalize(upVector); + + var right = NumericsVector3.Cross(upNormalized, forwardNormalized); + if (right.LengthSquared() <= epsilonSquared) + { + right = NumericsVector3.Cross(NumericsVector3.UnitY, forwardNormalized); + if (right.LengthSquared() <= epsilonSquared) + right = NumericsVector3.UnitX; + } + + right = NumericsVector3.Normalize(right); + var up = NumericsVector3.Cross(forwardNormalized, right); + var matrix = new NumericsMatrix4x4( + right.X, right.Y, right.Z, 0f, + up.X, up.Y, up.Z, 0f, + forwardNormalized.X, forwardNormalized.Y, forwardNormalized.Z, 0f, + 0f, 0f, 0f, 1f); + + return WrappedQuaternion.FromNumerics(NumericsQuaternion.CreateFromRotationMatrix(matrix)); + } + + [StructLayout(LayoutKind.Explicit)] + public struct WrappedVector3 + { + public const float kEpsilon = 1E-05f; + + [FieldOffset(0)] private NumericsVector3 _value; + [FieldOffset(0)] public float x; + [FieldOffset(4)] public float y; + [FieldOffset(8)] public float z; + + public WrappedVector3(float x, float y, float z) + { + this = default; + _value = new NumericsVector3(x, y, z); + } + + public float magnitude => _value.Length(); + public WrappedVector3 normalized => magnitude > kEpsilon ? this / magnitude : zero; + public static WrappedVector3 zero => new(0f, 0f, 0f); + public static WrappedVector3 operator /(WrappedVector3 a, float d) => FromNumerics(a._value / d); + + internal static NumericsVector3 ToNumerics(WrappedVector3 value) => value._value; + internal static WrappedVector3 FromNumerics(NumericsVector3 value) => new() { _value = value }; + } + + [StructLayout(LayoutKind.Explicit)] + public struct WrappedQuaternion + { + [FieldOffset(0)] private NumericsQuaternion _value; + [FieldOffset(0)] public float x; + [FieldOffset(4)] public float y; + [FieldOffset(8)] public float z; + [FieldOffset(12)] public float w; + + public static WrappedQuaternion identity => new(0f, 0f, 0f, 1f); + + public WrappedQuaternion(float x, float y, float z, float w) + { + this = default; + _value = new NumericsQuaternion(x, y, z, w); + } + + internal static WrappedQuaternion FromNumerics(NumericsQuaternion value) => new() { _value = value }; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.csproj new file mode 100644 index 00000000000000..09fb4358127787 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.csproj @@ -0,0 +1,13 @@ + + + None + True + + true + + + + + + + From 708c040f433010fd3653c88646d2cd875e272608 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 2 Jun 2026 22:32:13 +0200 Subject: [PATCH 2/5] JIT: simplify forwardsub guard and add morph FIELD_LIST handling for promoted SIMD locals Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/forwardsub.cpp | 23 ++++++++++++----------- src/coreclr/jit/morph.cpp | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index 8b1e317043de38..3bc8997491705c 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -791,23 +791,24 @@ bool Compiler::fgForwardSubStatement(Statement* stmt) dstVarDsc->SetIsMultiRegDest(); } - // A GT_LCL_VAR read of a promoted (non-DNER) SIMD local does not - // satisfy the post-lowering invariant in Lowering::CheckNode unless the - // use sits in a context that the lowering phase repairs (e.g. multi-reg-dest - // STORE_LCL_VAR or directly under a GT_RETURN). A GT_FIELD_LIST entry is - // not one of those contexts, so block the substitution. + // Don't forward sub a read of a promoted (non-DNER) SIMD local. Such a + // local has no maintained stack home, so a plain GT_LCL_VAR use of it only + // satisfies the post-lowering invariant in Lowering::CheckNode when it sits + // in a context that the lowering phase repairs (e.g. a multi-reg-dest + // STORE_LCL_VAR or directly under a GT_RETURN). Forward substitution can + // place the use in other contexts (e.g. under a GT_FIELD_LIST) that no + // later phase repairs, so just skip these locals. Dependently promoted + // (DNER) locals are fine since they keep a stack home (see also the + // promoted-struct handling in fgMorphHWIntrinsic and fgMorphSmpOp's + // GT_FIELD_LIST case). // if (fwdSubNode->OperIs(GT_LCL_VAR) && varTypeIsSIMD(fwdSubNode)) { LclVarDsc* const fwdSubVarDsc = lvaGetDesc(fwdSubNode->AsLclVar()); if (fwdSubVarDsc->lvPromoted && !fwdSubVarDsc->lvDoNotEnregister) { - GenTree* const parentNode = fsv.GetParentNode(); - if ((parentNode != nullptr) && parentNode->OperIs(GT_FIELD_LIST)) - { - JITDUMP(" promoted SIMD lcl var, parent is FIELD_LIST\n"); - return false; - } + JITDUMP(" promoted SIMD lcl var\n"); + return false; } } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index a4f09b64f6213a..b3155798dadf97 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12438,6 +12438,24 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) { use.SetNode(fgMorphTree(use.GetNode())); tree->gtFlags |= (use.GetNode()->gtFlags & GTF_ALL_EFFECT); + + // A promoted SIMD local read here has no maintained stack home + // and would fail Lowering::CheckNode, since a GT_FIELD_LIST is + // not a "special" use that a later phase repairs. Preserve the + // post-morph promoted-struct invariant by marking such an operand + // do-not-enregister (dependent promotion). This mirrors the + // handling in fgMorphHWIntrinsic. Note we restrict this to SIMD + // locals: a non-SIMD promoted struct is a legal FIELD_LIST + // operand (e.g. an independently promoted struct arg/return) and + // must keep its independent promotion. + // + GenTree* const operand = use.GetNode(); + if (operand->OperIs(GT_LCL_VAR) && varTypeIsSIMD(operand) && + lvaGetDesc(operand->AsLclVar())->lvPromoted) + { + lvaSetVarDoNotEnregister(operand->AsLclVar()->GetLclNum() + DEBUGARG(DoNotEnregisterReason::SimdUserForcesDep)); + } } break; From 4c4c43d99b586d800665cebbbf14d0983d3e8c85 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 6 Jul 2026 19:41:58 +0200 Subject: [PATCH 3/5] JIT: reword promoted-SIMD forwardsub/morph comments per review Reframe as a heuristic to avoid DNER when forward substituting whole-local uses into contexts (e.g. GT_FIELD_LIST) that don't support them, and drop the inaccurate stack-home / Lowering::CheckNode wording. No functional change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/forwardsub.cpp | 16 ++++++---------- src/coreclr/jit/morph.cpp | 15 ++++++--------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index 3bc8997491705c..57f38efc0819b7 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -791,16 +791,12 @@ bool Compiler::fgForwardSubStatement(Statement* stmt) dstVarDsc->SetIsMultiRegDest(); } - // Don't forward sub a read of a promoted (non-DNER) SIMD local. Such a - // local has no maintained stack home, so a plain GT_LCL_VAR use of it only - // satisfies the post-lowering invariant in Lowering::CheckNode when it sits - // in a context that the lowering phase repairs (e.g. a multi-reg-dest - // STORE_LCL_VAR or directly under a GT_RETURN). Forward substitution can - // place the use in other contexts (e.g. under a GT_FIELD_LIST) that no - // later phase repairs, so just skip these locals. Dependently promoted - // (DNER) locals are fine since they keep a stack home (see also the - // promoted-struct handling in fgMorphHWIntrinsic and fgMorphSmpOp's - // GT_FIELD_LIST case). + // Heuristic: don't forward substitute a whole read of a promoted SIMD local. + // Forward substitution can place the use in a context where a whole-local use + // is not supported (e.g. under a GT_FIELD_LIST), which would then require + // marking the local do-not-enregister, undoing its promotion. Conservatively + // skip these locals to avoid that. (Whole-local uses that do end up under a + // GT_FIELD_LIST are handled by DNER in fgMorphSmpOp, mirroring fgMorphHWIntrinsic.) // if (fwdSubNode->OperIs(GT_LCL_VAR) && varTypeIsSIMD(fwdSubNode)) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b3155798dadf97..3a0297052d620d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12439,15 +12439,12 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) use.SetNode(fgMorphTree(use.GetNode())); tree->gtFlags |= (use.GetNode()->gtFlags & GTF_ALL_EFFECT); - // A promoted SIMD local read here has no maintained stack home - // and would fail Lowering::CheckNode, since a GT_FIELD_LIST is - // not a "special" use that a later phase repairs. Preserve the - // post-morph promoted-struct invariant by marking such an operand - // do-not-enregister (dependent promotion). This mirrors the - // handling in fgMorphHWIntrinsic. Note we restrict this to SIMD - // locals: a non-SIMD promoted struct is a legal FIELD_LIST - // operand (e.g. an independently promoted struct arg/return) and - // must keep its independent promotion. + // A whole read of a promoted SIMD local is not a supported + // GT_FIELD_LIST operand. Preserve the post-morph promoted-struct + // invariant (see fgMorphHWIntrinsic) by marking it do-not-enregister + // (dependent promotion). Restrict this to SIMD locals: a non-SIMD + // promoted struct is a legal FIELD_LIST operand and must keep its + // independent promotion. // GenTree* const operand = use.GetNode(); if (operand->OperIs(GT_LCL_VAR) && varTypeIsSIMD(operand) && From aab0f60a16ec81985f164ae837cb6bd45be49e77 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 6 Jul 2026 20:01:17 +0200 Subject: [PATCH 4/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/coreclr/jit/forwardsub.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index 57f38efc0819b7..99cc620a5cffa1 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -794,10 +794,10 @@ bool Compiler::fgForwardSubStatement(Statement* stmt) // Heuristic: don't forward substitute a whole read of a promoted SIMD local. // Forward substitution can place the use in a context where a whole-local use // is not supported (e.g. under a GT_FIELD_LIST), which would then require - // marking the local do-not-enregister, undoing its promotion. Conservatively - // skip these locals to avoid that. (Whole-local uses that do end up under a - // GT_FIELD_LIST are handled by DNER in fgMorphSmpOp, mirroring fgMorphHWIntrinsic.) - // + // marking the local do-not-enregister, undoing its independent promotion. + // Conservatively skip these locals to avoid that. (Whole-local uses that do + // end up under a GT_FIELD_LIST are handled by do-not-enregister marking in + // fgMorphTree, mirroring fgMorphHWIntrinsic.) if (fwdSubNode->OperIs(GT_LCL_VAR) && varTypeIsSIMD(fwdSubNode)) { LclVarDsc* const fwdSubVarDsc = lvaGetDesc(fwdSubNode->AsLclVar()); From 5f6ab3326cbadc757c0cad917252464e2f851c86 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 7 Jul 2026 01:22:51 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/assertionprop.cpp | 11 +++-------- src/coreclr/jit/forwardsub.cpp | 10 +++------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 80aa21d06fffb6..7511729fff0205 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3489,14 +3489,9 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion, } } - // Don't propagate a promoted SIMD local into a whole-struct GT_LCL_VAR use. - // SIMD locals can be promoted into their lane/element fields (e.g. Quaternion - // -> 4 floats); the resulting "fully" promoted SIMD local has no maintained - // stack home and Lowering::CheckNode rejects a GT_LCL_VAR of it. Unlike - // generic struct copies (which Morph decomposes via MorphCopyBlock at the - // use site), substituting a promoted SIMD local can survive Morph and end - // up as e.g. a GT_FIELD_LIST entry that no later phase repairs. - // + // Do not propagate promoted locals if they are not DNER. + // This would require DNER'ing for many cases where the consumer + // does not support whole-local uses, such as GT_FIELD_LIST. if (tree->OperIs(GT_LCL_VAR) && varTypeIsSIMD(tree) && copyVarDsc->lvPromoted && !copyVarDsc->lvDoNotEnregister) { return nullptr; diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index 99cc620a5cffa1..c62672f24ee557 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -791,13 +791,9 @@ bool Compiler::fgForwardSubStatement(Statement* stmt) dstVarDsc->SetIsMultiRegDest(); } - // Heuristic: don't forward substitute a whole read of a promoted SIMD local. - // Forward substitution can place the use in a context where a whole-local use - // is not supported (e.g. under a GT_FIELD_LIST), which would then require - // marking the local do-not-enregister, undoing its independent promotion. - // Conservatively skip these locals to avoid that. (Whole-local uses that do - // end up under a GT_FIELD_LIST are handled by do-not-enregister marking in - // fgMorphTree, mirroring fgMorphHWIntrinsic.) + // Avoid forward substituting promoted locals if they are not DNER. + // This would require DNER'ing for many cases where the consumer + // does not support whole-local uses, such as GT_FIELD_LIST. if (fwdSubNode->OperIs(GT_LCL_VAR) && varTypeIsSIMD(fwdSubNode)) { LclVarDsc* const fwdSubVarDsc = lvaGetDesc(fwdSubNode->AsLclVar());