JIT: assert in CSE optimization

[erozenfeld]

The following test fails with checked x64 jit:

using System;
using System.Runtime.CompilerServices;

struct S
{
    public Program p;
    public int i;
}

struct T
{
    public S s;
}

class Program
{
    public T t;
    public T t1;

    [MethodImpl(MethodImplOptions.NoInlining)]
    int Test()
    {	
        if (this.t.s.p == this.t1.s.p)
        {
            return 0;
        }
        this.t1.s = this.t.s;
        int result = Helper(this.t.s);
        return result;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Helper(S s)
    {
        return s.i;
    }

    static int Main()
    {
        Program p = new Program();
        p.t.s.i = 100;
        p.t.s.p = p;

        return p.Test();
    }
}

The result is this assert:

Assert failure(PID 35752 [0x00008ba8], Thread: 21632 [0x5480]): Assertion failed 'IsCompatibleType(cseLclVarTyp, expTyp)' in 'Program:Test():int:this' (IL size 75)

    File: f:\coreclr9\src\jit\optcse.cpp Line: 2159
    Image: f:\coreclr9\bin\tests\Windows_NT.x64.Checked\Tests\Core_Root\CoreRun.exe

[erozenfeld]

The assert is about

N004 (  6,  5) CSE dotnet/coreclr#1 (use)[000020] *---G-------              \--*  IND       struct <l:$181, c:$240>
N003 (  3,  3) CSE dotnet/coreclr#2 (def)[000042] ------------                 \--*  ADD       byref  $100
N001 (  1,  1)              [000017] ------------                    +--*  LCL_VAR   ref    V00 this         u:1 $80
N002 (  1,  1)              [000041] ------------                    \--*  CNS_INT   long   8 field offset Fseq[t] $c0

and

N004 (  4,  4) CSE dotnet/coreclr#1 (def)[000005] *---G-------                 +--*  IND       ref    <l:$181, c:$1c0>
N003 (  2,  2)              [000036] -------N----                 |  \--*  ADD       byref  $100
N001 (  1,  1)              [000000] ------------                 |     +--*  LCL_VAR   ref    V00 this         u:1 $80
N002 (  1,  1)              [000035] ------------                 |     \--*  CNS_INT   long   8 field offset Fseq[t] $c0

The have the same liberal value numbers but different types (ref vs. struct).

The reason value numbering assigned them the same value numbers is that field sequence annotation is incorrect on the second tree:
the fact that the indirection returns the first field of the struct (a ref) and not the whole struct is lost.

The root cause is the morph transformation that transforms ADDR(IND(tree)) into tree and loses zero field tree annotations on the ADDR node:

[000004] ----G-------              *  ADDR      byref  Zero Fseq[p]
[000003] *---G+-N----              \--*  IND       struct
[000036] -----+------                 \--*  ADD       byref
[000000] -----+------                    +--*  LCL_VAR   ref    V00 this
[000035] -----+------                    \--*  CNS_INT   long   8 field offset Fseq[t]

is transformed to

[000036] -----+------              *  ADD       byref
[000000] -----+------              +--*  LCL_VAR   ref    V00 this
[000035] -----+------              \--*  CNS_INT   long   8 field offset Fseq[t]

[mikedn]

The root cause is the morph transformation that transforms ADDR(IND(tree)) into tree and loses zero field tree annotations on the ADDR node:

Those "tree annotations" are basically an accident waiting to happen. I would seriously consider getting rid of them.

[erozenfeld]

I agree but value numbering is currently very dependent on them so it's not easy to get rid of them. In this case it's easy to fix the morph transformation but cleaning up or replacing field sequence annotations should be on our list...

[mikedn]

Ah, I wasn't very clear. I was mostly referring to the specific case of "zero offset" field annotations that are stored in a map in the compiler object, not to the field annotations that are stored in nodes. And some similar "array info map" thing.

The field sequences themselves, hmm, I don't like them very much but they do the job for now. IMO the main problem with them is that they are attached to constant nodes and so they can be accidentally folded away. I think we could avoid such accidents by using special nodes to express field offset arithmetic, instead of the normal ADD and CNS_INT nodes.