JIT: Reevaluate args sorting

[jakobbotsch]

It is not clear that the args sorting the JIT does today is super beneficial. The idea is to evaluate expensive args early when more registers are available, but when I have tried to disable the logic entirely, I have seen positive diffs on x64.

Furthermore, we should reevaluate the order late args are placed in registers. We frequently hit cases where we place an argument into a register that contained a local that a subsequent arg needs to use. When this happens LSRA needs to spill, and since LSRA does not support spilling to registers, this becomes very expensive.

category:cq
theme:register-allocator
skill-level:expert
cost:medium
impact:medium

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

Issue Details

---

It is not clear that the args sorting the JIT does today is super beneficial. The idea is to evaluate expensive args early when more registers are available, but when I have tried to disable the logic entirely, I have seen positive diffs on x64.

Furthermore, we should reevaluate the order late args are placed in registers. We frequently hit cases where we place an argument into a register that contained a local that a subsequent arg needs to use. When this happens LSRA needs to spill, and since LSRA does not support spilling to registers, this becomes very expensive.

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

[jakobbotsch]

Couple of examples of the "subsequent argument uses placed register" problem:

#71161 (comment)
#75604 (comment)

[jakobbotsch]

When this happens LSRA needs to spill, and since LSRA does not support spilling to registers, this becomes very expensive

@kunalspathak Do you know if we have an existing issue for LSRA that covers this? There is both the notion of LSRA spill-temps that cannot be register allocated, but also the fact that we may home some locals on stack due to conflicts. I suppose the latter would be more live-range splitting related. Example:

private class C
{
    private int _num;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public void Foo(C c)
    {
        if (++_num > 50)
            return;

        c.Foo(this); // conflict: rcx -> rdx, rdx -> rcx
    }
}

Codegen today:

; Assembly listing for method C:Foo(C):this
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;  V00 this         [V00,T00] (  5,  4.50)     ref  ->  [rsp+30H]   this class-hnd single-def
;  V01 arg1         [V01,T01] (  3,  2.50)     ref  ->  rdx         class-hnd single-def
;  V02 loc0         [V02,T02] (  3,  3   )     int  ->  rax         single-def
;  V03 OutArgs      [V03    ] (  1,  1   )  lclBlk (32) [rsp+00H]   "OutgoingArgSpace"
;
; Lcl frame size = 40

G_M43131_IG01:              ;; offset=0000H
       4883EC28             sub      rsp, 40
                                                ;; size=4 bbWeight=1    PerfScore 0.25
G_M43131_IG02:              ;; offset=0004H
       8B4108               mov      eax, dword ptr [rcx+08H]
       FFC0                 inc      eax
       48894C2430           mov      gword ptr [rsp+30H], rcx
       894108               mov      dword ptr [rcx+08H], eax
       83F832               cmp      eax, 50
       7E05                 jle      SHORT G_M43131_IG04
                                                ;; size=18 bbWeight=1    PerfScore 5.50
G_M43131_IG03:              ;; offset=0016H
       4883C428             add      rsp, 40
       C3                   ret
                                                ;; size=5 bbWeight=0.50 PerfScore 0.62
G_M43131_IG04:              ;; offset=001BH
       488BCA               mov      rcx, rdx
       488B542430           mov      rdx, gword ptr [rsp+30H]
       3909                 cmp      dword ptr [rcx], ecx
       FF155D433000         call     [C:Foo(C):this]
       90                   nop
                                                ;; size=17 bbWeight=0.50 PerfScore 3.75
G_M43131_IG05:              ;; offset=002CH
       4883C428             add      rsp, 40
       C3                   ret
                                                ;; size=5 bbWeight=0.50 PerfScore 0.62

; Total bytes of code 49, prolog size 4, PerfScore 15.65, instruction count 16, allocated bytes for code 49 (MethodHash=ac505784) for method C:Foo(C):this
; ============================================================

Ideally we would spill this to another register instead.

(Note this example can be purely solved by better call args sorting, but the LSRA enhancements are more general)

[AndyAyersMS]

One way to approach this (which is similar to splitting) is to not allow any "long lived" enregisterables have hard physreg requirements, but always be copied to/from short-lived temps at ABI boundaries. Those short-lived temps would be constrained to the appropriate physregs. The longer-lived temps would be preferenced to the physregs they are moved to/from, but not constrained.

It can also help to model parameter passing in registers (and returns in multiple registers) as a "parallel assignment" rather than sequences of serial assignments, since these assignments can be freely reordered and seeing them all as a group can allow us to try and sort out the best order.

[kunalspathak]

I don't see any existing issue around this except #7999. I am trying to understand this space a little better and see what we can do here.

[kunalspathak]

I don't see any existing issue around this except #7999

This comment is somewhat related.

runtime/src/coreclr/jit/lsra.cpp

Lines 5164 to 5169 in b5f1ad4

	// Handle the case where this is a preassigned register (i.e. parameter).
	// We don't want to actually use the preassigned register if it's not
	// going to cover the lifetime - but we had to preallocate it to ensure
	// that it remained live.
	// TODO-CQ: At some point we may want to refine the analysis here, in case
	// it might be beneficial to keep it in this reg for PART of the lifetime

[jakobbotsch]

Linking #76017 where I had a go at this. It has overall good diffs but with a lot of mixed in regressions. I will put this in future but hope I get back to it soon, perhaps with a go at it from inside LSRA or lowering instead.