Runtime lookup clean up, enable for helper-based tail calls

[EgorBo]

Follow up to #81635

Clean up some dead code and enable runtime lookup expansion for helper-based tail-calls where we previously only used slow fallbacks.

[ghost]

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

Issue Details

---

Clean up some dead code and enable runtime lookup expansion for helper-based tail-calls where we previously only used slow fallbacks.

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

[jkotas]

This should be deleted from JIT/EE interface.

[EgorBo]

A couple of diffs in coreclr_tests where we have tail. callvirt and didn't use expansion previosly, e.g:

; Program:GenInterfaceForwardG
@@ -39,11 +42,26 @@ G_M7779_IG01: ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref,
mov rsi, r9
; gcrRegs +[rsi]
;; size=44 bbWeight=1 PerfScore 10.58
-G_M7779_IG02: ; bbWeight=1, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref+G_M7779_IG02: ; bbWeight=1, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref, isz+ mov rdx, qword ptr [rcx+38H]+ cmp qword ptr [rdx+10H], 32+ jle SHORT G_M7779_IG05+ ;; size=11 bbWeight=1 PerfScore 6.00+G_M7779_IG03: ; bbWeight=0.80, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref, isz+ mov rdx, qword ptr [rdx+20H]+ test rdx, rdx+ je SHORT G_M7779_IG05+ ;; size=9 bbWeight=0.80 PerfScore 2.60+G_M7779_IG04: ; bbWeight=0.64, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref, isz+ jmp SHORT G_M7779_IG06+ ;; size=2 bbWeight=0.64 PerfScore 1.28+G_M7779_IG05: ; bbWeight=0.36, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref
mov rdx, 0xD1FFAB1E ; global ptr
call CORINFO_HELP_RUNTIMEHANDLE_METHOD
; gcr arg pop 0
mov rdx, rax
+ ;; size=18 bbWeight=0.36 PerfScore 0.54+G_M7779_IG06: ; bbWeight=1, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref
mov rcx, rsi
; gcrRegs +[rcx]
mov r8, 0xD1FFAB1E ; token handle
@@ -67,8 +85,8 @@ G_M7779_IG02: ; bbWeight=1, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {},
; gcr arg pop 0
mov rax, gword ptr [rsp+30H]
; gcrRegs +[rax]
- ;; size=91 bbWeight=1 PerfScore 11.50-G_M7779_IG03: ; bbWeight=1, epilog, nogc, extend+ ;; size=73 bbWeight=1 PerfScore 10.00+G_M7779_IG07: ; bbWeight=1, epilog, nogc, extend
add rsp, 96
pop rbx
pop rsi
@@ -76,7 +94,7 @@ G_M7779_IG03: ; bbWeight=1, epilog, nogc, extend
ret ;; size=8 bbWeight=1 PerfScore 2.75
-; Total bytes of code 143

[jakobbotsch]

Nice! Any benchmarks?

BTW, doesn't the block layout look kind of odd here:

+G_M7779_IG03: ; bbWeight=0.80, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref, isz+ mov rdx, qword ptr [rdx+20H]+ test rdx, rdx+ je SHORT G_M7779_IG05+ ;; size=9 bbWeight=0.80 PerfScore 2.60+G_M7779_IG04: ; bbWeight=0.64, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref, isz+ jmp SHORT G_M7779_IG06+ ;; size=2 bbWeight=0.64 PerfScore 1.28+G_M7779_IG05: ; bbWeight=0.36, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref
mov rdx, 0xD1FFAB1E ; global ptr
call CORINFO_HELP_RUNTIMEHANDLE_METHOD
; gcr arg pop 0
mov rdx, rax
+ ;; size=18 bbWeight=0.36 PerfScore 0.54+G_M7779_IG06: ; bbWeight=1, gcrefRegs=00C0 {rsi rdi}, byrefRegs=0000 {}, byref

Couldn't this be jne G_M7779_IG06 directly from the hot block instead?

[jakobbotsch]

Quick benchmark:

publicstructForceHelper{publicintA,B,C,D,E,F,G;}publicinterfaceIFoo<T>{longCount(intcount,longsum,ForceHelperfh);}publicunsafeclassC<T>:IFoo<T>{publiclongCount(intiterations,longsum,ForceHelperfh){if(iterations==0)returnsum;ForceHelperlocal=fh;local.A=30;IL.Push(this);IL.Push(iterations-1);IL.Push(sum+iterations);IL.Push(local);IL.Emit.Tail();IL.Emit.Callvirt(newMethodRef(typeof(IFoo<T>),"Count"));returnIL.Return<long>();}}

Method	Job	Toolchain	iterations	Mean	Error	StdDev	Ratio
Count	DefaultJob	.NET 7	1000000	205.30 ms	2.596 ms	2.428 ms	1.00
Count	Job-CEFCSM	pr	1000000	47.73 ms	0.306 ms	0.271 ms	0.23

🚀

[EgorBo]

Quick benchmark:

Thanks for checking - I was puzzled on how to properly benchmark it 🙂.

Couldn't this be jne G_M7779_IG06 directly from the hot block instead?

We emit block layout like this:

// prevBb(BBJ_NONE): [weight: 1.0]// ...//// nullcheckBb(BBJ_COND): [weight: 1.0]// if (*fastPathValue == null)// goto fallbackBb;//// fastPathBb(BBJ_ALWAYS): [weight: 0.8]// rtLookupLcl = *fastPathValue;// goto block;//// fallbackBb(BBJ_NONE): [weight: 0.2]// rtLookupLcl = HelperCall();//// block(...): [weight: 1.0]// use(rtLookupLcl);

We probably need to re-order fastPathBb and fallbackBb