JIT: morph's HW intrinsic operand walk ignores GTF_REVERSE_OPS, eliding a null check before its side effect

[EgorBo]

Description

fgMorphHWIntrinsic walks a HW intrinsic's operands in physical order rather than execution order, so local assertion propagation can apply an assertion generated by a later-executing operand to an earlier-executing one. When the earlier operand is a call that needs a null check, the check is removed and the call executes on a null receiver.

Reproduction Steps

x64 Windows, main, FullOpts (Release or Checked clrjit):

using System;
using System.Runtime.CompilerServices;
using System.Runtime.Intrinsics;

class Program
{
    static int s_calls;

    class C
    {
        public int Field = 3;

        [MethodImpl(MethodImplOptions.NoInlining)]
        public int GetValue()
        {
            s_calls++;
            return 7;
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
    static Vector128<int> Test(C c) =>
        Vector128.ConcatUpperUpper(Vector128.Create(c.GetValue()), Vector128.Create(c.Field));

    static void Main()
    {
        try
        {
            Test(null);
        }
        catch (NullReferenceException)
        {
        }

        Console.WriteLine($"GetValue calls: {s_calls} (expected 0)");
    }
}

Expected behavior

GetValue is an instance method invoked on a null receiver, so it must never run — the NullReferenceException has to be raised before it.

GetValue calls: 0 (expected 0)

This is also what DOTNET_JitMinOpts=1 prints.

Actual behavior

GetValue calls: 1 (expected 0)

The call executes and mutates static state before the NullReferenceException is eventually raised.

Analysis

gtNewSimdConcatNode emits MoveHighToLow(op2, op1) and sets GTF_REVERSE_OPS, so the physical operand order is right-then-left while the execution order is left-then-right:

https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/gentree.cpp#L28194

fgMorphHWIntrinsic iterates the operands with tree->UseEdges():

https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/morph.cpp#L11542

Since tree is a GenTreeHWIntrinsic*, this binds to GenTreeMultiOp::UseEdges(), which is explicitly documented to ignore GTF_REVERSE_OPS:

https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/gentree.h#L6373

So morph processes the right operand first. Its c.Field load generates the local assertion V01 != null, and that assertion is then used to clear GTF_CALL_NULLCHECK on the call, which actually executes first:

https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/assertionprop.cpp#L5362

Relevant JitDump excerpt (Morph - Global):

               [000007] --CXG---R--       \--*  HWINTRINSIC simd16 16 float MoveHighToLow
               [000006] ---XG------          +--*  HWINTRINSIC simd16 16 int Create
               [000005] n--XG------          |  \--*  IND       int
               [000004] ---X-------          |     \--*  FIELD_ADDR byref  Receiver:Field
               [000003] -----------          |        \--*  LCL_VAR   ref    V01 arg0
               [000002] --CXG------          \--*  HWINTRINSIC simd16 16 int Create
               [000001] --CXG------             \--*  CALL nullcheck int    Receiver:GetValue():int:this

GenTreeNode creates assertion:
               [000005] ---XG+-----       *  IND       int
In BB01 New Local #01 lclvar V01 != null
Non-null assertion prop for tree [000000] in BB01:

Note the R (reverse ops) on [000007]: [000002]/[000001] execute before [000006]/[000005], yet the assertion from [000005] is applied to the call.

Resulting codegen — no null check anywhere, and the field load is what eventually faults, after the call:

IN0001: 00000C mov      rcx, rbx
IN0002: 00000F call     [Receiver:GetValue():int:this]
IN0003: 000015 vpbroadcastd xmm0, eax
IN0004: 00001B vpbroadcastd xmm1, dword ptr [rbx+0x08]
IN0005: 000021 vmovhlps xmm0, xmm1, xmm0

Using the execution-order-aware base iterator (tree->GenTree::UseEdges()) in fgMorphHWIntrinsic restores the correct order. Other assertion consumers I checked already honor execution order (global assertion prop's threaded node list, VNAssertionPropVisitor, AssertionsAccumulator in range check, and LocalAddressVisitor, all via UseExecutionOrder = true).

Affected versions

Only main, as far as I can tell. The Concat* APIs were added in #129627, and the other import-time SetReverseOp producer, gtNewSimdShuffleVariableNode, is not reachable at morph time because variable-index Shuffle defers expansion to rationalize (which asserts the flag is clear). The fgMorphHWIntrinsic issue itself is older and was latent.

[dotnet-policy-service]

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