RyuJIT doesn't elide frames for passtrough methods with multiple calls

[SingleAccretion]

Description

Consider the following three methods:

public static int TwoCalls(int a)
{
    var one = One(a);
    if (one > 0)
    {
        return one;
    }

    return Two(a);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static int One(int a) => a;

[MethodImpl(MethodImplOptions.NoInlining)]
public static int Two(int a) => a;

Today (tested on 3ae36b5 via Disasmo), the following code is generated for TwoCalls on x64:

G_M61010_IG01:
       push     rsi
       sub      rsp, 32
       mov      esi, ecx
						;; bbWeight=1    PerfScore 1.50
G_M61010_IG02:
       mov      ecx, esi
       call     ThrowawayTesting.Program:One(int):int
       test     eax, eax
       jle      SHORT G_M61010_IG04
						;; bbWeight=1    PerfScore 2.50
G_M61010_IG03:
       add      rsp, 32
       pop      rsi
       ret      
						;; bbWeight=0.50 PerfScore 0.88
G_M61010_IG04:
       mov      ecx, esi
						;; bbWeight=0.50 PerfScore 0.12
G_M61010_IG05:
       add      rsp, 32
       pop      rsi
       jmp      ThrowawayTesting.Program:Two(int):int

As can be seen, for each path, parameter in rcx is first moved to rsi and then moved back to rcx, causing the method to also save and restore rsi (which is undesirable, especially when more parameters are involved).

[AndyAyersMS]

I suspect this is a "feature" of the linear scan allocator. Linearizing lifetimes makes it looks like one of the calls interferes with the other.

@kunalspathak PTAL.

[kunalspathak]

@AndyAyersMS is right. When building the intervals, we assign rcx to the parameter 'a', but when we see that there is a call, we kill the callee-saved registers and update the preference of the corresponding interval to exclude them. There is potentially a CQ item to not do it in certain scenario, but I believe the cost of that would be higher than the benefit it provides.

I was able to see similar behavior for msvc as well.
https://godbolt.org/z/vd6KP6

For clang/gcc, noinline attribute is not working well.

Let me know if that answers the question. If yes, then I will go ahead and close the issue as there might not be an immediate action for this.

[SingleAccretion]

When building the intervals, we assign rcx to the parameter 'a', but when we see that there is a call, we kill the callee-saved registers and update the preference of the corresponding interval to exclude them.

I see. That is a bit unfortunate for methods like these:

switch (*chars)
{
    case 'A': result = ParseA(chars); break;
    case 'B': result = ParseB(chars); break;
    ...
}

Let me know if that answers the question. If yes, then I will go ahead and close the issue as there might not be an immediate action for this.

Since that's the case, sure.