Behavior changes with optimizations on x86

[RossNordby]

Description

Attempting to run the given snippet of code while targeting x86 with optimizations enabled produces invalid results. Disabling optimizations or targeting x64 avoids the issue.

The repro demonstrates an infinite loop. Other uses have exhibited access violations.

(Sorry, didn't have time to squeeze the repro down further!)

Reproduction Steps

Call Reprotest.Test():

using System.Numerics;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;

namespace x86Repro;

unsafe class Reprotest
{
    [StructLayout(LayoutKind.Explicit)]
    public struct NodeChild
    {
        [FieldOffset(0)]
        public Vector3 Min;
        [FieldOffset(12)]
        public int Index;
        [FieldOffset(16)]
        public Vector3 Max;
        [FieldOffset(28)]
        public int LeafCount;
    }
    [StructLayout(LayoutKind.Explicit)]
    public struct Node
    {
        [FieldOffset(0)]
        public NodeChild A;
        [FieldOffset(32)]
        public NodeChild B;
    }

    public struct Buffer<T> where T : unmanaged
    {
        public T* Memory;
        public int Length;
        public Buffer(T* memory, int length)
        {
            Memory = memory;
            Length = length;
        }

        public ref T this[int index]
        {
            get
            {
                Debug.Assert(index >= 0 && index < Length, "hey that's no good");
                return ref Memory[index];
            }
        }
    }

    public struct TreeRay
    {
        public Vector3 OriginOverDirection;
        public float MaximumT;
        public Vector3 InverseDirection;

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static void CreateFrom(Vector3 origin, Vector3 direction, float maximumT, out TreeRay treeRay)
        {
            treeRay.InverseDirection = new Vector3(direction.X < 0 ? -1 : 1, direction.Y < 0 ? -1 : 1, direction.Z < 0 ? -1 : 1) / Vector3.Max(new Vector3(1e-15f), Vector3.Abs(direction));
            treeRay.MaximumT = maximumT;
            treeRay.OriginOverDirection = origin * treeRay.InverseDirection;
        }
    }

    public struct Reprotree
    {
        public int LeafCount;
        public int NodeCount;
        public Buffer<Node> Nodes;

        //NOTE: Removing aggressive inlining avoids the issue.
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public unsafe static bool Intersects(Vector3 min, Vector3 max, TreeRay* ray, out float t)
        {
            var t0 = min * ray->InverseDirection - ray->OriginOverDirection;
            var t1 = max * ray->InverseDirection - ray->OriginOverDirection;
            var tExit = Vector3.Max(t0, t1);
            var tEntry = Vector3.Min(t0, t1);
            var earliestExit = Vector4.Min(Vector4.Min(new Vector4(ray->MaximumT), new Vector4(tExit.X)), Vector4.Min(new Vector4(tExit.Y), new Vector4(tExit.Z))).X;
            t = Vector4.Max(Vector4.Max(new Vector4(tEntry.X), Vector4.Zero), Vector4.Max(new Vector4(tEntry.Y), new Vector4(tEntry.Z))).X;
            return t <= earliestExit;
        }

        public static int Encode(int index)
        {
            return -1 - index;
        }

        //NOTE: Disabling optimization avoids the issue.
        //[MethodImpl(MethodImplOptions.NoOptimization)]
        public readonly unsafe void Sweep(int nodeIndex, Vector3 expansion, TreeRay* treeRay, Buffer<int> stack)
        {
            Debug.Assert((nodeIndex >= 0 && nodeIndex < NodeCount) || (Encode(nodeIndex) >= 0 && Encode(nodeIndex) < LeafCount));
            Debug.Assert(LeafCount >= 2, "This implementation assumes all nodes are filled.");

            int stackEnd = 0;
            while (true)
            {
                if (nodeIndex < 0)
                {
                    Console.WriteLine($"Entering leaf.");
                    //This is actually a leaf node.
                    //Leaves have no children; have to pull from the stack to get a new target.
                    if (stackEnd == 0)
                        return;
                    nodeIndex = stack[--stackEnd];
                }
                else
                {
                    //if (nodeIndex < 0 || nodeIndex >= NodeCount)
                    //{
                    //    Console.WriteLine($"BAD index {nodeIndex} versus {NodeCount}");
                    //    Console.ReadLine();
                    //}
                    ref var node = ref Nodes[nodeIndex];
                    Console.WriteLine($"Entering node. ChildA, B: {node.A.Index}, {node.B.Index}");
                    var minA = node.A.Min - expansion;
                    var maxA = node.A.Max + expansion;
                    var aIntersected = Intersects(minA, maxA, treeRay, out var tA);
                    var minB = node.B.Min - expansion;
                    var maxB = node.B.Max + expansion;
                    var bIntersected = Intersects(minB, maxB, treeRay, out var tB);

                    if (aIntersected)
                    {
                        if (bIntersected)
                        {
                            //Visit the earlier AABB intersection first.
                            if (tA < tB)
                            {
                                //Console.WriteLine($"Using inline node index A: {node.A.Index}");
                                nodeIndex = node.A.Index;
                                stack[stackEnd++] = node.B.Index;
                            }
                            else
                            {
                                //Console.WriteLine($"Using inline node index B: {node.B.Index}");
                                nodeIndex = node.B.Index;
                                stack[stackEnd++] = node.A.Index;
                            }
                        }
                        else
                        {
                            //Single intersection cases don't require an explicit stack entry.
                            //Console.WriteLine($"Using inline node index A: {node.A.Index}");
                            nodeIndex = node.A.Index;
                        }
                    }
                    else if (bIntersected)
                    {
                        //Console.WriteLine($"Using inline node index B: {node.B.Index}");
                        nodeIndex = node.B.Index;
                    }
                    else
                    {
                        //No intersection. Need to pull from the stack to get a new target.
                        //Console.WriteLine($"need to yoink from stack.");
                        if (stackEnd == 0)
                            return;
                        nodeIndex = stack[--stackEnd];
                        //Console.WriteLine($"Popped new node index: {nodeIndex}");
                    }
                }
            }
        }
    }

    public static void Test()
    {
        const int stackSize = 256;
        var stackMemory = stackalloc int[stackSize];
        var stack = new Buffer<int>(stackMemory, stackSize);

        var tree = new Reprotree();
        var node = new Node();
        node.A = new NodeChild { LeafCount = 1, Index = -1, Min = new Vector3(-5), Max = new Vector3(5) };
        node.B = new NodeChild { LeafCount = 1, Index = -2, Min = new Vector3(-5), Max = new Vector3(5) };
        tree.Nodes = new Buffer<Node>(&node, 1);
        tree.NodeCount = 1;
        tree.LeafCount = 2;
        TreeRay.CreateFrom(Vector3.Zero, new Vector3(0, 1, 0), 1, out var treeRay);
        tree.Sweep(0, new Vector3(5), &treeRay, stack);

    }
}

Expected behavior

Calling Reprotest.Test() should print:

Entering node. ChildA, B: -1, -2
Entering leaf.
Entering leaf.

and then return.

Actual behavior

Targeting x86 with optimizations prints an infinite loop:

Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
...

Other similar use cases also sometimes triggered access violations.

Regression?

Seems to occur on .NET 6, 7, and 8, assuming I didn't accidentally let it roll forward to a later SDK during those tests.

Known Workarounds

No response

Configuration

Observed on .NET 8, Windows 11, x86 target architecture. Only happens in release mode with optimizations enabled. x64 target works as expected with optimizations.

Other information

Relatively small changes to the function can change the behavior. Removing the Console.WriteLine calls or not inlining the Intersects test both make it work as expected.

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Attempting to run the given snippet of code while targeting x86 with optimizations enabled produces invalid results. Disabling optimizations or targeting x64 avoids the issue.

The repro demonstrates an infinite loop. Other uses have exhibited access violations.

(Sorry, didn't have time to squeeze the repro down further!)

Reproduction Steps

Call Reprotest.Test():

using System.Numerics;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;

namespace x86Repro;

unsafe class Reprotest
{
    [StructLayout(LayoutKind.Explicit)]
    public struct NodeChild
    {
        [FieldOffset(0)]
        public Vector3 Min;
        [FieldOffset(12)]
        public int Index;
        [FieldOffset(16)]
        public Vector3 Max;
        [FieldOffset(28)]
        public int LeafCount;
    }
    [StructLayout(LayoutKind.Explicit)]
    public struct Node
    {
        [FieldOffset(0)]
        public NodeChild A;
        [FieldOffset(32)]
        public NodeChild B;
    }

    public struct Buffer<T> where T : unmanaged
    {
        public T* Memory;
        public int Length;
        public Buffer(T* memory, int length)
        {
            Memory = memory;
            Length = length;
        }

        public ref T this[int index]
        {
            get
            {
                Debug.Assert(index >= 0 && index < Length, "hey that's no good");
                return ref Memory[index];
            }
        }
    }

    public struct TreeRay
    {
        public Vector3 OriginOverDirection;
        public float MaximumT;
        public Vector3 InverseDirection;

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static void CreateFrom(Vector3 origin, Vector3 direction, float maximumT, out TreeRay treeRay)
        {
            treeRay.InverseDirection = new Vector3(direction.X < 0 ? -1 : 1, direction.Y < 0 ? -1 : 1, direction.Z < 0 ? -1 : 1) / Vector3.Max(new Vector3(1e-15f), Vector3.Abs(direction));
            treeRay.MaximumT = maximumT;
            treeRay.OriginOverDirection = origin * treeRay.InverseDirection;
        }
    }

    public struct Reprotree
    {
        public int LeafCount;
        public int NodeCount;
        public Buffer<Node> Nodes;

        //NOTE: Removing aggressive inlining avoids the issue.
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public unsafe static bool Intersects(Vector3 min, Vector3 max, TreeRay* ray, out float t)
        {
            var t0 = min * ray->InverseDirection - ray->OriginOverDirection;
            var t1 = max * ray->InverseDirection - ray->OriginOverDirection;
            var tExit = Vector3.Max(t0, t1);
            var tEntry = Vector3.Min(t0, t1);
            var earliestExit = Vector4.Min(Vector4.Min(new Vector4(ray->MaximumT), new Vector4(tExit.X)), Vector4.Min(new Vector4(tExit.Y), new Vector4(tExit.Z))).X;
            t = Vector4.Max(Vector4.Max(new Vector4(tEntry.X), Vector4.Zero), Vector4.Max(new Vector4(tEntry.Y), new Vector4(tEntry.Z))).X;
            return t <= earliestExit;
        }

        public static int Encode(int index)
        {
            return -1 - index;
        }

        //NOTE: Disabling optimization avoids the issue.
        //[MethodImpl(MethodImplOptions.NoOptimization)]
        public readonly unsafe void Sweep(int nodeIndex, Vector3 expansion, TreeRay* treeRay, Buffer<int> stack)
        {
            Debug.Assert((nodeIndex >= 0 && nodeIndex < NodeCount) || (Encode(nodeIndex) >= 0 && Encode(nodeIndex) < LeafCount));
            Debug.Assert(LeafCount >= 2, "This implementation assumes all nodes are filled.");

            int stackEnd = 0;
            while (true)
            {
                if (nodeIndex < 0)
                {
                    Console.WriteLine($"Entering leaf.");
                    //This is actually a leaf node.
                    //Leaves have no children; have to pull from the stack to get a new target.
                    if (stackEnd == 0)
                        return;
                    nodeIndex = stack[--stackEnd];
                }
                else
                {
                    //if (nodeIndex < 0 || nodeIndex >= NodeCount)
                    //{
                    //    Console.WriteLine($"BAD index {nodeIndex} versus {NodeCount}");
                    //    Console.ReadLine();
                    //}
                    ref var node = ref Nodes[nodeIndex];
                    Console.WriteLine($"Entering node. ChildA, B: {node.A.Index}, {node.B.Index}");
                    var minA = node.A.Min - expansion;
                    var maxA = node.A.Max + expansion;
                    var aIntersected = Intersects(minA, maxA, treeRay, out var tA);
                    var minB = node.B.Min - expansion;
                    var maxB = node.B.Max + expansion;
                    var bIntersected = Intersects(minB, maxB, treeRay, out var tB);

                    if (aIntersected)
                    {
                        if (bIntersected)
                        {
                            //Visit the earlier AABB intersection first.
                            if (tA < tB)
                            {
                                //Console.WriteLine($"Using inline node index A: {node.A.Index}");
                                nodeIndex = node.A.Index;
                                stack[stackEnd++] = node.B.Index;
                            }
                            else
                            {
                                //Console.WriteLine($"Using inline node index B: {node.B.Index}");
                                nodeIndex = node.B.Index;
                                stack[stackEnd++] = node.A.Index;
                            }
                        }
                        else
                        {
                            //Single intersection cases don't require an explicit stack entry.
                            //Console.WriteLine($"Using inline node index A: {node.A.Index}");
                            nodeIndex = node.A.Index;
                        }
                    }
                    else if (bIntersected)
                    {
                        //Console.WriteLine($"Using inline node index B: {node.B.Index}");
                        nodeIndex = node.B.Index;
                    }
                    else
                    {
                        //No intersection. Need to pull from the stack to get a new target.
                        //Console.WriteLine($"need to yoink from stack.");
                        if (stackEnd == 0)
                            return;
                        nodeIndex = stack[--stackEnd];
                        //Console.WriteLine($"Popped new node index: {nodeIndex}");
                    }
                }
            }
        }
    }

    public static void Test()
    {
        const int stackSize = 256;
        var stackMemory = stackalloc int[stackSize];
        var stack = new Buffer<int>(stackMemory, stackSize);

        var tree = new Reprotree();
        var node = new Node();
        node.A = new NodeChild { LeafCount = 1, Index = -1, Min = new Vector3(-5), Max = new Vector3(5) };
        node.B = new NodeChild { LeafCount = 1, Index = -2, Min = new Vector3(-5), Max = new Vector3(5) };
        tree.Nodes = new Buffer<Node>(&node, 1);
        tree.NodeCount = 1;
        tree.LeafCount = 2;
        TreeRay.CreateFrom(Vector3.Zero, new Vector3(0, 1, 0), 1, out var treeRay);
        tree.Sweep(0, new Vector3(5), &treeRay, stack);

    }
}

Expected behavior

Calling Reprotest.Test() should print:

Entering node. ChildA, B: -1, -2
Entering leaf.
Entering leaf.

and then return.

Actual behavior

Targeting x86 with optimizations prints an infinite loop:

Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
Entering node. ChildA, B: -1, -2
Entering leaf.
...

Other similar use cases also sometimes triggered access violations.

Regression?

Seems to occur on .NET 6, 7, and 8, assuming I didn't accidentally let it roll forward to a later SDK during those tests.

Known Workarounds

No response

Configuration

Observed on .NET 8, Windows 11, x86 target architecture. Only happens in release mode with optimizations enabled. x64 target works as expected with optimizations.

Other information

Relatively small changes to the function can change the behavior. Removing the Console.WriteLine calls or not inlining the Intersects test both make it work as expected.

Author:	RossNordby
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[AndyAyersMS]

FYI @dotnet/jit-contrib

[JulieLeeMSFT]

@EgorBo, PTAL.

[EgorBo]

Thanks @RossNordby I managed to reduce the repro down to:

using System.Numerics;
using System.Runtime.CompilerServices;

unsafe class Reprotest
{
    [MethodImpl(MethodImplOptions.NoInlining | 
        MethodImplOptions.AggressiveOptimization)]
    public static void Sweep(int ecx, int* edx, Vector3 stack12)
    {
        while (true)
        {
            Console.WriteLine(ecx);
            if (ecx == -1)
            {
                ecx = edx[0];
            }
            else if (!float.IsNaN(stack12.X))
            {
                edx[0] = -1;
                ecx = -1;
            }
        }
    }

    [MethodImpl(MethodImplOptions.NoOptimization)]
    public static void Main()
    {
        int x = 42;
        Sweep(1, &x, Vector3.Zero);
    }
}

it prints different output in Release and Debug on x86 target.

; Assembly listing for method Reprotest:Sweep(int,uint,System.Numerics.Vector3) (FullOpts)
; Emitting BLENDED_CODE for X86 with AVX512 - Windows
; FullOpts code
; optimized code
; ebp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 1 single block inlinees; 0 inlinees without PGO data
; invoked as altjit
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  6, 34   )     int  ->  esi        
;  V01 arg1         [V01,T01] (  4, 18   )     int  ->  edi         single-def
;  V02 arg2         [V02,T03] (  1,  8   )  simd12  ->  [ebp+0x08]  single-def <System.Numerics.Vector3>
;  V03 tmp0         [V03,T02] (  3, 48   )   float  ->  mm1         "Inlining Arg"
;
; Lcl frame size = 0
G_M49595_IG01:  ;; offset=0x0000
       push     ebp
       mov      ebp, esp
       push     edi
       push     esi
       mov      esi, ecx
       mov      edi, edx
G_M49595_IG02:
       mov      ecx, esi
       call     [System.Console:WriteLine(int)]
       cmp      esi, -1
       jne      SHORT G_M49595_IG03
       mov      esi, dword ptr [edi]
       jmp      SHORT G_M49595_IG02
G_M49595_IG03:
       vmovups  xmm0, xmmword ptr [ebp+0x08]
       vmovaps  xmm1, xmm0
       vucomiss xmm1, xmm1
       jp       SHORT G_M49595_IG04
       mov      dword ptr [edi], -1
       mov      esi, -1
       vmovups  xmmword ptr [ebp+0x08], xmm0
       jmp      SHORT G_M49595_IG02
G_M49595_IG04:
       vmovups  xmmword ptr [ebp+0x08], xmm0
       jmp      SHORT G_M49595_IG02
; Total bytes of code 66, prolog size 5, PerfScore 194.00, instruction count 22, allocated bytes for code 66 (MethodHash=0c463e44) for method Reprotest:Sweep(int,uint,System.Numerics.Vector3) (FullOpts)
; ============================================================

[EgorBo]

looks like it gets overwritten by vmovups xmmword ptr [ebp+8], xmm0

I presume the 0x08 offset is incorrect here and the Vector3 was supposed to be accessed from +12 ?

cc @dotnet/jit-contrib if this rings a bell. Perhaps, the issue is in X86Classifier::Classify @jakobbotsch ?

[jakobbotsch]

The problem is incorrect handling of this resolution move inserted for stack12:

Splitting edge from BB04 to BB02; adding BB06
setting likelihood of BB06 -> BB02 to 1
   BB06 bottom (BB04->BB02): move V02 from mm0 to STK (Critical)
N001 (  3,  2) [000048] ----------Z                   u48 =    LCL_VAR   simd12<System.Numerics.Vector3> V02 arg2          mm0 REG mm0

The spill code is generating a 16 byte spill, but it needs to only store 12 bytes when lvaMapSimd12ToSimd16 returns false.

On a separate topic it's silly in the first place that we are generating any spill here considering the parameter is never redefined.