Unnecessary(?) runtime lookups for constrained interface calls

[jakobbotsch]

The behavior change in #73606 is due to a JIT bug no longer triggering, which was bisected to be a side effect of #66887.

@markples tracked down that we are in fact seeing new runtime lookups due to this change. We are not quite sure whether these are necessary or not. Simple example:

using System;
using System.Runtime.CompilerServices;

public class Program
{
 public static void Main()
 {
 Console.WriteLine(CallFoo(new C()));
 }

 [MethodImpl(MethodImplOptions.NoInlining)]
 private static int CallFoo<T>(T val) where T : IFace
 {
 return val.Foo();
 }
}

public interface IFace
{
 int Foo();
}

public class C : IFace
{
 public int Foo() => 0;
}

Codegen before #66887:

; Assembly listing for method Program:CallFoo(System.__Canon):int
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;* V00 TypeCtx [V00 ] ( 0, 0 ) long -> zero-ref single-def
; V01 arg0 [V01,T00] ( 3, 3 ) ref -> rdx ld-addr-op class-hnd single-def
; V02 OutArgs [V02 ] ( 1, 1 ) lclBlk (32) [rsp+00H] "OutgoingArgSpace"
;
; Lcl frame size = 40

G_M39006_IG01:
 sub rsp, 40
 ;; size=4 bbWeight=1 PerfScore 0.25
G_M39006_IG02:
 mov rcx, rdx
 mov r11, 0xD1FFAB1E
 call [r11]IFace:Foo():int:this
 nop
 ;; size=17 bbWeight=1 PerfScore 3.75
G_M39006_IG03:
 add rsp, 40
 ret
 ;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 26, prolog size 4, PerfScore 7.85, instruction count 7, allocated bytes for code 26 (MethodHash=972067a1) for method Program:CallFoo(System.__Canon):int
; ============================================================

Codegen on current main:

; Assembly listing for method Program:CallFoo(System.__Canon):int
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
; V00 TypeCtx [V00,T00] ( 5, 4.25) long -> rcx single-def
; V01 arg0 [V01,T02] ( 3, 3 ) ref -> rsi ld-addr-op class-hnd single-def
; V02 OutArgs [V02 ] ( 1, 1 ) lclBlk (32) [rsp+00H] "OutgoingArgSpace"
; V03 tmp1 [V03,T01] ( 4, 5 ) long -> r11 "spilling Runtime Lookup tree"
;* V04 tmp2 [V04 ] ( 0, 0 ) long -> zero-ref "VirtualCall with runtime lookup"
; V05 cse0 [V05,T03] ( 3, 2.25) long -> r11 "CSE - aggressive"
;
; Lcl frame size = 48

G_M39006_IG01:
 push rsi
 sub rsp, 48
 mov qword ptr [rsp+28H], rcx
 mov rsi, rdx
 ;; size=13 bbWeight=1 PerfScore 2.50
G_M39006_IG02:
 mov rdx, qword ptr [rcx+56]
 mov r11, qword ptr [rdx+16]
 test r11, r11
 je SHORT G_M39006_IG04
 ;; size=13 bbWeight=1 PerfScore 5.25
G_M39006_IG03:
 jmp SHORT G_M39006_IG05
 ;; size=2 bbWeight=0.25 PerfScore 0.50
G_M39006_IG04:
 mov rdx, 0xD1FFAB1E ; global ptr
 call CORINFO_HELP_RUNTIMEHANDLE_METHOD
 mov r11, rax
 ;; size=18 bbWeight=0.25 PerfScore 0.38
G_M39006_IG05:
 mov rcx, rsi
 call [r11]
 nop
 ;; size=7 bbWeight=1 PerfScore 3.50
G_M39006_IG06:
 add rsp, 48
 pop rsi
 ret
 ;; size=6 bbWeight=1 PerfScore 1.75

; Total bytes of code 59, prolog size 10, PerfScore 19.78, instruction count 18, allocated bytes for code 59 (MethodHash=972067a1) for method Program:CallFoo(System.__Canon):int
; ============================================================

Aside from the perf regression, fixing this should return the behavior of #73606 back to what it was previously. Then we plan to fix the underlying Roslyn and JIT issues (#73615) in .NET 8.

cc @trylek @davidwrighton @jaredpar @VSadov

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

Issue Details

---

The behavior change in #73606 is due to a JIT bug no longer triggering, which was bisected to be caused by #66887.

@markples tracked down that we are in fact seeing new runtime lookups due to this change. We are not quite sure whether these are necessary or not. Simple example:

using System;
using System.Runtime.CompilerServices;

public class Program
{
 public static void Main()
 {
 Console.WriteLine(CallFoo(new C()));
 }

 [MethodImpl(MethodImplOptions.NoInlining)]
 private static int CallFoo<T>(T val) where T : IFace
 {
 return val.Foo();
 }
}

public interface IFace
{
 int Foo();
}

public class C : IFace
{
 public int Foo() => 0;
}

Codegen before #66887:

; Assembly listing for method Program:CallFoo(System.__Canon):int
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;* V00 TypeCtx [V00 ] ( 0, 0 ) long -> zero-ref single-def
; V01 arg0 [V01,T00] ( 3, 3 ) ref -> rdx ld-addr-op class-hnd single-def
; V02 OutArgs [V02 ] ( 1, 1 ) lclBlk (32) [rsp+00H] "OutgoingArgSpace"
;
; Lcl frame size = 40

G_M39006_IG01:
 sub rsp, 40
 ;; size=4 bbWeight=1 PerfScore 0.25
G_M39006_IG02:
 mov rcx, rdx
 mov r11, 0xD1FFAB1E
 call [r11]IFace:Foo():int:this
 nop
 ;; size=17 bbWeight=1 PerfScore 3.75
G_M39006_IG03:
 add rsp, 40
 ret
 ;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 26, prolog size 4, PerfScore 7.85, instruction count 7, allocated bytes for code 26 (MethodHash=972067a1) for method Program:CallFoo(System.__Canon):int
; ============================================================

Codegen on current main:

; Assembly listing for method Program:CallFoo(System.__Canon):int
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
; V00 TypeCtx [V00,T00] ( 5, 4.25) long -> rcx single-def
; V01 arg0 [V01,T02] ( 3, 3 ) ref -> rsi ld-addr-op class-hnd single-def
; V02 OutArgs [V02 ] ( 1, 1 ) lclBlk (32) [rsp+00H] "OutgoingArgSpace"
; V03 tmp1 [V03,T01] ( 4, 5 ) long -> r11 "spilling Runtime Lookup tree"
;* V04 tmp2 [V04 ] ( 0, 0 ) long -> zero-ref "VirtualCall with runtime lookup"
; V05 cse0 [V05,T03] ( 3, 2.25) long -> r11 "CSE - aggressive"
;
; Lcl frame size = 48

G_M39006_IG01:
 push rsi
 sub rsp, 48
 mov qword ptr [rsp+28H], rcx
 mov rsi, rdx
 ;; size=13 bbWeight=1 PerfScore 2.50
G_M39006_IG02:
 mov rdx, qword ptr [rcx+56]
 mov r11, qword ptr [rdx+16]
 test r11, r11
 je SHORT G_M39006_IG04
 ;; size=13 bbWeight=1 PerfScore 5.25
G_M39006_IG03:
 jmp SHORT G_M39006_IG05
 ;; size=2 bbWeight=0.25 PerfScore 0.50
G_M39006_IG04:
 mov rdx, 0xD1FFAB1E ; global ptr
 call CORINFO_HELP_RUNTIMEHANDLE_METHOD
 mov r11, rax
 ;; size=18 bbWeight=0.25 PerfScore 0.38
G_M39006_IG05:
 mov rcx, rsi
 call [r11]
 nop
 ;; size=7 bbWeight=1 PerfScore 3.50
G_M39006_IG06:
 add rsp, 48
 pop rsi
 ret
 ;; size=6 bbWeight=1 PerfScore 1.75

; Total bytes of code 59, prolog size 10, PerfScore 19.78, instruction count 18, allocated bytes for code 59 (MethodHash=972067a1) for method Program:CallFoo(System.__Canon):int
; ============================================================

Aside from the perf regression, fixing this should return the behavior of #73606 back to what it was previously. Then we plan to fix the underlying Roslyn and JIT issues (#73615) in .NET 8.

cc @trylek @davidwrighton @jaredpar

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

[markples]

As an experiment, I tried undoing the exactContextNeedsRuntimeLookup logic in 1313fda.

d:\r\runtime\src\tests>git diff
diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp
index 4837e51ab89..59afbfe55b7 100644
--- a/src/coreclr/vm/jitinterface.cpp
+++ b/src/coreclr/vm/jitinterface.cpp
@@ -4955,10 +4955,11 @@ void CEEInfo::getCallInfo(
 MethodDesc * pTargetMD = pMDAfterConstraintResolution;
 DWORD dwTargetMethodAttrs = pTargetMD->GetAttrs();

- pResult->exactContextNeedsRuntimeLookup = (!constrainedType.IsNull() && constrainedType.IsCanonicalSubtype());
+ pResult->exactContextNeedsRuntimeLookup = FALSE;

 if (pTargetMD->HasMethodInstantiation())
 {
+ pResult->exactContextNeedsRuntimeLookup = (!constrainedType.IsNull() && constrainedType.IsCanonicalSubtype());
 pResult->contextHandle = MAKE_METHODCONTEXT(pTargetMD);
 if (pTargetMD->GetMethodTable()->IsSharedByGenericInstantiations() || TypeHandle::IsCanonicalSubtypeInstantiation(pTargetMD->GetMethodInstantiation()))
 {

This restored the constrained.callvirt behavior. I expected it to break Loader\classloader\StaticVirtualMethods\DiamondShape\svm_diamondshape_r.ilproj since that test was added in 1313fda, but it apparently did not. I'd need to understand this better. (note - if this is the correct change, there's a cleaner undo)

[trylek]

Thanks Jakob and Mark for investigating this. Rereading the code it seems to me that the problem is caused by the fact that the runtime lookup should only kick in for static virtual methods that failed to resolve at JIT time (fIsStaticVirtualMethods && !fResolvedConstraint) so that it doesn't affect pre-existing code paths. I'm not yet fully clear on why reverting the logic altogether doesn't break the diamond shape test, I'll debug that locally. At this point I'm inclined to believe that the easiest fix is to add the two conditions I mentioned to the unconditional line setting exactContextNeedsRuntimeLookup before the conditional block. I can send the fix for PR later today unless you're already working on it.

[jakobbotsch]

I can send the fix for PR later today unless you're already working on it.

Feel free to send the fix.

[AndyAyersMS]

Seems odd that we didn't note any perf lab regressions from the initial change.

[jakobbotsch]

Seems odd that we didn't note any perf lab regressions from the initial change.

I agree, I opened dotnet/performance#2553 for that.