-
Notifications
You must be signed in to change notification settings - Fork 5.5k
JIT: don't propagate promoted struct LCL_VAR into FIELD_LIST uses #128375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
EgorBo
merged 6 commits into
dotnet:main
from
EgorBo:fix-128373-promoted-struct-fieldlist
Jul 7, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
df448be
JIT: don't propagate promoted SIMD LCL_VAR into FIELD_LIST uses
Copilot b56f0b7
Merge branch 'main' into fix-128373-promoted-struct-fieldlist
EgorBo 392fdba
JIT: simplify forwardsub guard and add morph FIELD_LIST handling for …
EgorBo 4769589
JIT: reword promoted-SIMD forwardsub/morph comments per review
EgorBo 706fc68
Potential fix for pull request finding
EgorBo 69386c9
Apply suggestions from code review
EgorBo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 }; | ||
| } | ||
| } |
13 changes: 13 additions & 0 deletions
13
src/tests/JIT/Regression/JitBlue/Runtime_128373/Runtime_128373.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <DebugType>None</DebugType> | ||
| <Optimize>True</Optimize> | ||
| <!-- Needed for CLRTestEnvironmentVariable --> | ||
| <RequiresProcessIsolation>true</RequiresProcessIsolation> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <Compile Include="$(MSBuildProjectName).cs" /> | ||
| <CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="1" /> | ||
| <CLRTestEnvironmentVariable Include="DOTNET_TieredPGO" Value="1" /> | ||
| </ItemGroup> | ||
| </Project> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.