JIT cannot devirtualize interface call in trivial cases

[sakno]

Description

JIT cannot devirtualize interface call in very trivial cases.

Source code:

using System;
using System.Runtime.CompilerServices;

public static class C {
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    private static T As<T>(T obj) where T : class => obj;
    
    public static int Cmp(String str) => As<IComparable<string>>(str).CompareTo(string.Empty);
    
    public static int Cmp2(String str) => str.CompareTo(string.Empty);
    
    public static int Cmp3(String str) => ((IComparable<string>)str).CompareTo(string.Empty);
    
    public static int Cmp4(IComparable<string> str) => str.CompareTo(string.Empty);
}

Output assembly:

C.Cmp(System.String)
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: mov edx, [0x8d72018]
    L0009: call dword ptr [0x11317000]
    L000f: pop ebp
    L0010: ret

C.Cmp2(System.String)
    L0000: mov edx, [0x8d72018]
    L0006: cmp [ecx], ecx
    L0008: push 0
    L000a: call System.String.Compare(System.String, System.String, System.StringComparison)
    L000f: ret

C.Cmp3(System.String)
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: mov edx, [0x8d72018]
    L0009: call dword ptr [0x11317004]
    L000f: pop ebp
    L0010: ret

C.Cmp4(System.IComparable`1<System.String>)
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: mov edx, [0x8d72018]
    L0009: call dword ptr [0x11317008]
    L000f: pop ebp
    L0010: ret

Cmp2 is correctly devirtualized and inlined. I'm expecting that the assembly code should be the same for all cases except Cmp4.

Configuration

.NET 6.0.101
Ubuntu 20.04.3 5.4.0-91-generic x86_64

Regression?

Probably yes, but I didn't check.

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

Issue Details

---

Description

JIT cannot devirtualize interface call in very trivial cases.

Source code:

using System;
using System.Runtime.CompilerServices;

public static class C {
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    private static T As<T>(T obj) where T : class => obj;
    
    public static int Cmp(String str) => As<IComparable<string>>(str).CompareTo(string.Empty);
    
    public static int Cmp2(String str) => str.CompareTo(string.Empty);
    
    public static int Cmp3(String str) => ((IComparable<string>)str).CompareTo(string.Empty);
    
    public static int Cmp4(IComparable<string> str) => str.CompareTo(string.Empty);
}

Output assembly:

C.Cmp(System.String)
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: mov edx, [0x8d72018]
    L0009: call dword ptr [0x11317000]
    L000f: pop ebp
    L0010: ret

C.Cmp2(System.String)
    L0000: mov edx, [0x8d72018]
    L0006: cmp [ecx], ecx
    L0008: push 0
    L000a: call System.String.Compare(System.String, System.String, System.StringComparison)
    L000f: ret

C.Cmp3(System.String)
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: mov edx, [0x8d72018]
    L0009: call dword ptr [0x11317004]
    L000f: pop ebp
    L0010: ret

C.Cmp4(System.IComparable`1<System.String>)
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: mov edx, [0x8d72018]
    L0009: call dword ptr [0x11317008]
    L000f: pop ebp
    L0010: ret

Cmp2 is correctly devirtualized. I'm expecting that the assembly code should be the same for all cases except Cmp4.

Configuration

.NET 6.0.101
Ubuntu 20.04.3 5.4.0-91-generic x86_64

Regression?

Probably yes, but I didn't check.

Author:	sakno
Assignees:	-
Labels:	tenet-performance, area-CodeGen-coreclr, untriaged
Milestone:	-

[EgorBo]

Cmp2 and Cmp3 have the same codegen in .NET 7.0
Cmp1 is a known JIT's limitation - we don't re-visit calls we marked as non-devirtualizable/non-inlinineable candidates after we inlined some inner calls in the same method.

But it does happen when Dynamic PGO is turned on.

[sakno]

Any chance to backport JIT behavior as you described to .NET 6 ?

[EgorBo]

Any chance to backport JIT behavior as you described to .NET 6 ?

Dynamic PGO? you can enable it in .NET 6.0, see https://gist.github.com/EgorBo/dc181796683da3d905a5295bfd3dd95b#how-can-i-try-it-on-my-production

[sakno]

No, not Dynamic PGO. I'm talking about to have the same codegen for Cmp2 and Cmp3 in .NET 6.

[EgorBo]

No, not Dynamic PGO. I'm talking about to have the same codegen for Cmp2 and Cmp3 in .NET 6.

Unlikely, the bar to backport changes to .NET 6.0 is very high to include an optimization.

[AndyAyersMS]

I think we should be able to get the Cmp case via late devirtualization, but for some reason the interface class we end up with doesn't seem to be compatible with string and so we fail to do so now. Need to analyze further.

*** CMP

impDevirtualizeCall: Trying to devirtualize interface call: (late)

    class for 'this' is System.String [final] (attrib 20040010)
    base method is System.IComparable`1[__Canon]::CompareTo
--- base class is interface
--- no derived method: object class could not be cast to interface class
    Class not final or exact
No guarded devirt during late devirtualization

*** CMP3

impDevirtualizeCall: Trying to devirtualize interface call: (early)
    class for 'this' is System.String [final] (attrib 20040010)
    base method is System.IComparable`1[__Canon]::CompareTo
--- base class is interface
    devirt to System.String::CompareTo -- final class

[AndyAyersMS]

Seems like the issue is that in the Cmp case we end up losing the generic context for the As (which should be somehow associated with the return value) and so are trying to cast from System.String to IComparable<_Canon> sans context, and this fails in the runtime, whereas with Cmp3 we have appropriate context and are casting to IComparable<System.String>.

When we propagate a return value out of a generic method to the non-generic root, we ideally should be able to determine the proper unshared type for that value. This might work roughly as follows:

• in impReturnInstruction, if the inlinee has a generics context, record it on a new field in the GT_RET_EXPR
• when swapping out the GT_RET_EXPR for the return value tree, somehow attach the context to the return value tree. if we're updating the this of virtual or interface call. Note we could attach it to the parent call perhaps.... would need to be a bit careful as this is just the context for this and not for the body of the method. But since we can't inline after this, perhaps it would be ok.
• then pass this context to the call to impDevirtualizeCall we make in fgLateDevirtualization.

[AndyAyersMS]

Another perhaps more robust option would be to retype the return value, if it's a ref type, and we have a context.