JIT: Peel offsets from spilled addresses for store decomposition

[jakobbotsch]

For a block operation like

▌  ASG       struct (copy)
├──▌  BLK       struct<System.Nullable`1, 8>
│  └──▌  ADD       byref
│     ├──▌  LCL_VAR   ref    V00 this
│     └──▌  CNS_INT   long   40 Fseq[<unknown field>]
└──▌  LCL_VAR   struct<System.Nullable`1, 8>(P) V06 tmp2
└──▌    bool   V06.<unknown class>:hasValue (offs=0x00) -> V12 tmp8          (last use)
└──▌    int    V06.<unknown class>:value (offs=0x04) -> V13 tmp9          (last use)

We would previously spill the ADD(V00, 40) to a local and end up producing:

▌  COMMA     void
├──▌  COMMA     void
│  ├──▌  ASG       byref
│  │  ├──▌  LCL_VAR   byref  V15 tmp11
│  │  └──▌  ADD       byref
│  │     ├──▌  LCL_VAR   ref    V00 this
│  │     └──▌  CNS_INT   long   40 Fseq[<unknown field>]
│  └──▌  ASG       bool
│     ├──▌  IND       bool
│     │  └──▌  LCL_VAR   byref  V15 tmp11
│     └──▌  LCL_VAR   bool   V12 tmp8
└──▌  ASG       int
   ├──▌  IND       int
   │  └──▌  ADD       byref
   │     ├──▌  LCL_VAR   byref  V15 tmp11
   │     └──▌  CNS_INT   long   4
   └──▌  LCL_VAR   int    V13 tmp9

Instead of this we can include the offsets with the field offsets. In many cases that allows us to avoid any temporary locals, while in other cases it usually allows us to avoid an instruction in favor of two larger containments. In this case we instead end up with:

▌  COMMA     void
├──▌  ASG       bool
│  ├──▌  IND       bool
│  │  └──▌  ADD       byref
│  │     ├──▌  LCL_VAR   ref    V00 this
│  │     └──▌  CNS_INT   long   40 Fseq[<unknown field>]
│  └──▌  LCL_VAR   bool   V12 tmp8
└──▌  ASG       int
   ├──▌  IND       int
   │  └──▌  ADD       byref
   │     ├──▌  LCL_VAR   ref    V00 this
   │     └──▌  CNS_INT   long   44 Fseq[<unknown field>, 4]
   └──▌  LCL_VAR   int    V13 tmp9

[ghost]

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

Issue Details

---

For a block operation like

▌  ASG       struct (copy)
├──▌  BLK       struct<System.Nullable`1, 8>
│  └──▌  ADD       byref
│     ├──▌  LCL_VAR   ref    V00 this
│     └──▌  CNS_INT   long   40 Fseq[<unknown field>]
└──▌  LCL_VAR   struct<System.Nullable`1, 8>(P) V06 tmp2
└──▌    bool   V06.<unknown class>:hasValue (offs=0x00) -> V12 tmp8          (last use)
└──▌    int    V06.<unknown class>:value (offs=0x04) -> V13 tmp9          (last use)

We would previously spill the ADD(V00, 40) to a local and end up producing:

▌  COMMA     void
├──▌  COMMA     void
│  ├──▌  ASG       byref
│  │  ├──▌  LCL_VAR   byref  V15 tmp11
│  │  └──▌  ADD       byref
│  │     ├──▌  LCL_VAR   ref    V00 this
│  │     └──▌  CNS_INT   long   40 Fseq[<unknown field>]
│  └──▌  ASG       bool
│     ├──▌  IND       bool
│     │  └──▌  LCL_VAR   byref  V15 tmp11
│     └──▌  LCL_VAR   bool   V12 tmp8
└──▌  ASG       int
   ├──▌  IND       int
   │  └──▌  ADD       byref
   │     ├──▌  LCL_VAR   byref  V15 tmp11
   │     └──▌  CNS_INT   long   4
   └──▌  LCL_VAR   int    V13 tmp9

Instead of this we can include the offsets with the field offsets. In many cases that allows us to avoid any temporary locals, while in other cases it usually allows us to avoid an instruction in favor of two larger containments. In this case we instead end up with:

▌  COMMA     void
├──▌  ASG       bool
│  ├──▌  IND       bool
│  │  └──▌  ADD       byref
│  │     ├──▌  LCL_VAR   ref    V00 this
│  │     └──▌  CNS_INT   long   40 Fseq[<unknown field>]
│  └──▌  LCL_VAR   bool   V12 tmp8
└──▌  ASG       int
   ├──▌  IND       int
   │  └──▌  ADD       byref
   │     ├──▌  LCL_VAR   ref    V00 this
   │     └──▌  CNS_INT   long   44 Fseq[<unknown field>, 4]
   └──▌  LCL_VAR   int    V13 tmp9

Author:	jakobbotsch
Assignees:	jakobbotsch
Labels:	area-CodeGen-coreclr
Milestone:	-

[jakobbotsch]

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jakobbotsch]

Fuzzlyn failures are #66579. jitstress failures were #83658, #84006 (and recently disabled by #85936).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @EgorBo @SingleAccretion. Diffs.

Regressions are because:

1. Missed CSEs in some cases now that the ADD ends up being "contained" for each field
2. New CSEs in some cases because we can better recognize the field accesses across different instances of block morphing
3. Larger encoding required in some cases for each field access
4. Worse register allocation with the new CSEs or due to the fewer locals

[tannergooding]

Just noting the larger containment "might" be worse in some cases as the optimization manuals cover that there is a "cheap" and "expensive" form of LEA addressing.

In particular, memory addressing that has 1 or 2 operands can be dispatched on 2 different ports (1 or 5) and takes 1 cycle, while those that have three operands can only dispatch on a single port (1) and takes 3 cycles.
-- This was the case even as recently as Cascade Lake. It changed in Ice Lake and later to have 4 ports (0, 1, 5, or 6) and to always be 1 cycle.
-- On Zen, there remains a 2 vs 4 port split and the expensive is "1-2" cycles, with the exact expense depending on the inputs

The "expensive" form in particular is:

• Use all three operands base, scaled-index, and offset (sometimes called displacement)
• Use base + index where the base is ebp/rbp or r13
• Use RIP-relative addressing
• Use 16-bit addressing

Don't think its particularly relevant, but might be something for us to keep in mind.

[AndyAyersMS]

We could anti-preference the allocator from using RBP/R13 for LEA base registers.

[jakobbotsch]

If larger offsets are not actually more costly (except for the [rcx] vs [rcx+something] case), then indeed I don't think it's going to affect much here.
In some cases this will change e.g. add rcx, 16 followed by mov reg, [rcx] into mov reg, [rcx+16], but in most cases the previous removed instruction will be a lea anyway with its own addressing.

The rbp/r13 case might be hit, but I don't think the new IR is intrinsically worse; it is switching one base local for a different base local, so whether or not that ends up introducing or removing r13/rbp as the base register depends on luck/unluck.

[tannergooding]

We could anti-preference the allocator from using RBP/R13 for LEA base registers.

This sounds like a good/small up for grabs issue.

In some cases this will change e.g. add rcx, 16 followed by mov reg, [rcx] into mov reg, [rcx+16], but in most cases the previous removed instruction will be a lea anyway with its own addressing.

That case should be fine as its just base + offset or scaled-index + offset. The case that would potentially hurt is:

- add rcx, 16
- mov reg, [rcx+rdx]    ; base + scaled-index
+ mov reg, [rcx+rdx+16] ; base + scaled-index + offset

[jakobbotsch]

We could anti-preference the allocator from using RBP/R13 for LEA base registers.

This sounds like a good/small up for grabs issue.

In some cases this will change e.g. add rcx, 16 followed by mov reg, [rcx] into mov reg, [rcx+16], but in most cases the previous removed instruction will be a lea anyway with its own addressing.

That case should be fine as its just base + offset or scaled-index + offset. The case that would potentially hurt is:

- add rcx, 16
- mov reg, [rcx+rdx]    ; base + scaled-index
+ mov reg, [rcx+rdx+16] ; base + scaled-index + offset

I see, in that case I don't think this PR should hit problems since it will always be the "base + offset" form.

[jakobbotsch]

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jakobbotsch]

Fuzzlyn failure is a macOS spurious failure that did not repro when it tried to reduce it. NAOT timeouts are known.