JIT: Use faster write barriers when we know the target address is on the heap

[EgorBo]

Closes #97534

TL;DR: JIT has an optimization that tries to simplify/remove write barriers, but that optimization happens too late to rely on VN/SSA/Assertions, so CSE can easily ruin it by moving addresses or values to locals and forcing that opt to give up and use the checked write barrier. This PR assists that optimization from assertion prop (#97901 did it for nongc objects as values for such indirs), Example:

public struct Slot
{
    public object Item;
    public int SequenceNumber;
}

static void Test(Slot[] arr, object o)
{
    arr[0].Item = o;
    arr[0].SequenceNumber = 1;
}

Codegen for Test:

; Method LdtokenRepro:Test(Slot[],System.Object) (FullOpts)
G_M35662_IG01:  ;; offset=0x0000
       push     rbx
       sub      rsp, 32
G_M35662_IG02:  ;; offset=0x0005
       cmp      dword ptr [rcx+0x08], 0
       jbe      SHORT G_M35662_IG04
       lea      rbx, bword ptr [rcx+0x10]
       mov      rcx, rbx
-      call     CORINFO_HELP_CHECKED_ASSIGN_REF
+      call     CORINFO_HELP_ASSIGN_REF
       mov      dword ptr [rbx+0x08], 1
G_M35662_IG03:  ;; offset=0x001E
       add      rsp, 32
       pop      rbx
       ret      
G_M35662_IG04:  ;; offset=0x0024
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
; Total bytes of code: 42

[ghost]

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

Issue Details

---

Closes #97534

TL;DR: JIT has an optimization that tries to simplify/remove write barriers, but that optimization happens to late to rely on VN/SSA/Assertions, so CSE can easily ruin it by moving addresses or values to locals and forcing that opt to give up and use the checked write barrier. This PR assists that optimization from assertion prop, Example:

public struct Slot
{
    public object Item;
    public int SequenceNumber;
}

static void Test(Slot[] arr, object o)
{
    arr[0].Item = o;
    arr[0].SequenceNumber = 1;
}

Codegen for Test:

; Method LdtokenRepro:Test(Slot[],System.Object) (FullOpts)
G_M35662_IG01:  ;; offset=0x0000
       push     rbx
       sub      rsp, 32
G_M35662_IG02:  ;; offset=0x0005
       cmp      dword ptr [rcx+0x08], 0
       jbe      SHORT G_M35662_IG04
       lea      rbx, bword ptr [rcx+0x10]
       mov      rcx, rbx
-      call     CORINFO_HELP_CHECKED_ASSIGN_REF
+      call     CORINFO_HELP_ASSIGN_REF
       mov      dword ptr [rbx+0x08], 1
G_M35662_IG03:  ;; offset=0x001E
       add      rsp, 32
       pop      rbx
       ret      
G_M35662_IG04:  ;; offset=0x0024
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
; Total bytes of code: 42

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

[EgorBo]

@AndyAyersMS @jakobbotsch cc @SingleAccretion @dotnet/jit-contrib PTAL. Diffs. Diffs aren't too big because CORINFO_HELP_CHECKED_ASSIGN_REF -> CORINFO_HELP_ASSIGN_REF change has 0 size diff, but should improve perf since the latter doesn't need to check whether pointer targets heap or not (two comparisons). Most size diffs come from PtrToLoc -> NoWritebarrier. A small TP regression due to GetVNFunc in a loop (the regression was much bigger before I minimized it), but, hopefully, faster write barriers are worth it.

Closes #97534

[EgorBo]

@MihuBot

[EgorBo]

ping @AndyAyersMS - I presume it should help you with CSE work since CORINFO_HELP_ASSIGN_REF and CORINFO_HELP_CHECKED_ASSIGN_REF have the same asm and PerfScore, so CSE changes migth silently regress them.

I analyzed jit-diff on top of BCL and this PR removed ~2400 checked write barriers.

[EgorBo]

NOTE: this has an assumption that "addressOfLocal + addressOfHeap" is UB

[EgorBo]

Made it a bit more conservative (it didn't affect diffs) by requiring the 2nd argument to be a non-handle constant. So, if we see address being GT_ADD(op1, op2) and one of the arguments is either address-of-local or address-within-heap, the other argument must be a non-handle constant.

[kunalspathak]

LGTM. Thanks!

[AndyAyersMS]

LGTM. Thanks for fixing this.

We could / should have perf score reflect the cost of helper calls (likewise with gtCostEx).

[EgorBo]

We could / should have perf score reflect the cost of helper calls (likewise with gtCostEx).

I agree, I assume currently all unknown calls have the same cost (assuming same arguments)? Do we want to make all helpers cheaper than unknown calls or more expensive? I presume this could result in some diffs

[EgorBo]

Failures are known + PR passed all checks before adding debug JITDUMPs