JIT: Assertion failed 'cond == test->AsOp()->gtOp1' during 'Update flow graph early pass'

[jakobbotsch]

// Generated by Fuzzlyn v1.6 on 2024-06-16 15:10:23
// Run on X64 Windows
// Seed: 6886681321438315426
// Reduced from 30.1 KiB to 0.3 KiB in 00:01:11
// Hits JIT assert in Release:
// Assertion failed 'cond == test->AsOp()->gtOp1' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Update flow graph early pass' (IL size 27; hash 0xade6b36b; FullOpts)
// 
//     File: D:\a\_work\1\s\src\coreclr\jit\fgopt.cpp Line: 5447
// 
using System.Runtime.CompilerServices;

public class Program
{
    public static void Main()
    {
        ulong[] vr0 = default(ulong[]);
        if (0 > vr0[0])
        {
            vr0 = vr0;
        }
        else
        {
            vr0 = new ulong[]{0};
        }

        vr0[0] = vr0[0];
    }
}

cc @amanasifkhalid, looks like there's still some of these cases hanging around.

[dotnet-policy-service]

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

[jakobbotsch]

Actually this looks more like a coincidence. I bisected this to #103382. @tannergooding can you please take a look? There is an invariant in the JIT until lowering that the child node of GT_JTRUE must be a relop, so it is illegal to fold those nodes.

[tannergooding]

There is an invariant in the JIT until lowering that the child node of GT_JTRUE must be a relop, so it is illegal to fold those nodes.

@jakobbotsch, so basically we can't ever fold relops (GT_LE, GT_GE, GT_LT, GT_GT, GT_EQ, or GT_NE) if one operand is constant and the other has a side effect?

I don't think we have an easy way to check the use here (and may not even have one if the folding is happening in early import), but it seems like this might be missing some optimization opportunities if we can't do this folding since we won't see that the JTRUE is always taken, right?

[jakobbotsch]

There is an invariant in the JIT until lowering that the child node of GT_JTRUE must be a relop, so it is illegal to fold those nodes.

@jakobbotsch, so basically we can't ever fold relops (GT_LE, GT_GE, GT_LT, GT_GT, GT_EQ, or GT_NE) if one operand is constant and the other has a side effect?

There is GTF_RELOP_JMP_USED, though I'm not sure how up to date it is at this point.

I don't think we have an easy way to check the use here (and may not even have one if the folding is happening in early import), but it seems like this might be missing some optimization opportunities if we can't do this folding since we won't see that the JTRUE is always taken, right?

Morph folds these by ensuring the branch is also folded and the flow graph updated. It might be that you can fix the issue here by teaching the morph part to recognize this new result of folding that might be a comma-wrapped constant (it will need to do similar extraction of side effects into a new statement before the JTRUE).

[jakobbotsch]

Actually it's not morph, but rather import that needs to be taught to recognize this new shape of folded relops.

This needs to be updated to handle the new cases:

runtime/src/coreclr/jit/importer.cpp

Lines 7303 to 7310 in e679b59

	/* Fold comparison if we can */
	op1 = gtFoldExpr(op1);
	/* Try to fold the really simple cases like 'iconst *, ifne/ifeq'*/
	/* Don't make any blocks unreachable in import only mode */
	if (op1->gtOper == GT_CNS_INT)