JIT poor codegen for calculating field offsets of structs

[john-h-k]

Description

The JIT doesn't generate good assembly when calculating (what should be) constant fields offsets.
Example sharplab

Configuration

.NET 5, master branch on sharplab and I repro'd it locally with what i believe is a reasonably up to date copy of the runtime (i don't think it is more than a week old)

Analysis

I'm aware the JIT has to throw for null in the first null case, but they show it has the ability to resolve it to a constant (it emits it as a constant load of 4 there, although right after throwing an NRE). It doesn't seem to succeed doing this with either a local (even when SkipLocalsInit is enabled) or just with a pointer (where I would expect identical codegen to the null case, except comparing the pointer for null rather than immediately NREing).

[module: SkipLocalsInit]

public struct Example
{
    public byte Zero;

    private byte _1, _2, _3;

    public byte Four;
}

// good codegen :)! ... but instant null ref ex
public int OffsetAsNull()
{
    Example* p = null;
    return (int)(&p->Four - &p->Zero);
}

// bad codegen
public int OffsetAsNotNullWithLocal()
{
    Example local;
    Example* p = &local;
    return (int)(&p->Four - &p->Zero);
}

// bad codegen
public int OffsetAsNotNullWithParam(Example* p)
{
    return (int)(&p->Four - &p->Zero);
}

generates

C.OffsetAsNull()
    L0000: xor eax, eax
    L0002: cmp [eax], eax ; NRE
    L0004: mov eax, 4 ; constant offset load
    L0009: ret

C.OffsetAsNotNullWithLocal()
    L0000: sub esp, 8
    L0003: lea eax, [esp]
    L0006: cmp [eax], eax
    L0008: lea edx, [eax+4]
    L000b: sub edx, eax
    L000d: mov eax, edx
    L000f: add esp, 8
    L0012: ret

C.OffsetAsNotNullWithParam(Example*)
    L0000: cmp [edx], edx
    L0002: lea eax, [edx+4]  
    L0005: sub eax, edx ; it sets eax to edx + 4, then subs, but doesn't fold that to a constant?
    L0007: ret

category:cq
theme:basic-cq
skill-level:intermediate
cost:medium
impact:small

[EgorBo]

.NET 5, master branch on sharplab

Yeah it's quite confusing, but "master" on sharplab means "master version of Roslyn + .NET Core 3.1 x86 JIT" (not even x64)
It confuses so many people so many times so we should ask sharplab authors to make it clearer 🙂

I'd recommend to either use BenchmarkDotNet's disasm + .NET 5 or use COMPlus_JitDisasm=method (preferable)

[john-h-k]

I repro'd it locally with what i believe is a reasonably up to date copy of the runtime (i don't think it is more than a week old)

I did check it with my local copy of the runtime too 😄 , just can't do it on a freshly pulled copy right now because I am having some issues building it

edit: fixed the build issue and rechecked. There is no change in codegen except the JIT optimises the part which throws an NRE from xor; cmp to a single mov 😆 @EgorBo

[AndyAyersMS]

@john-h-k thanks. Is this representative of something you've run into somewhere, or just something you happened to notice?

Marking this as future as we're battening down the hatches for 5.0.

[john-h-k]

no problem 😄. Ran into it in a marshalling scenario where I have to relatively frequently offset some pointers based off of a field offset

[AndyAyersMS]

Actually marking as future.

[MichalStrehovsky]

I ran into pretty much the same thing in #9791 - we can probably merge these issues because they look similar.

The workaround with null is nice. I'll probably use that.

[john-h-k]

While implementing the no. nullcheck prefix for ldfld, i realised if it supported ldflda then this would allow this to work. ECMA doesn't say no. nullcheck ldflda is valid for some reason (i really am not sure why), but when i allowed it, I got the perfect constant folded codegen 🎉

G_M11044_IG01:
                                                ;; bbWeight=1    PerfScore 0.00
G_M11044_IG02:
       B804000000           mov      eax, 4
                                                ;; bbWeight=1    PerfScore 0.25
G_M11044_IG03:
       C3                   ret
                                                ;; bbWeight=1    PerfScore 1.00```

[BruceForstall]

C.OffsetAsNotNullWithLocal() could lose the extra mov with slightly better register allocation.

Both C.OffsetAsNotNullWithLocal() and C.OffsetAsNotNullWithParam() look the same to the JIT after morph:

*  RETURN    int
\--*  CAST      int <- long
   \--*  SUB       long
      +--*  COMMA     long
      |  +--*  NULLCHECK byte
      |  |  \--*  LCL_VAR   long   V01 arg1
      |  \--*  ADD       long
      |     +--*  LCL_VAR   long   V01 arg1
      |     \--*  CNS_INT   long   4 field offset Fseq[Four]
      \--*  LCL_VAR   long   V01 arg1          Zero Fseq[Zero]

Presumably this pattern could be recognized and folded if it's common enough to warrant. The NULLCHECK node slightly complicates it. The JIT assumes that if you're creating a byref (or native pointer) that it will be dereferenced, so it conservatively creates the null check. Here, presumably we don't need the null checks.