-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regressions in Devirtualization.EqualityComparer #88667
Comments
Probably caused by #88006. cc @MichalPetryka |
@jkotas Could this maybe be the removal of the ByteEqualityComparer? Nothing else comes to my mind here. |
Are you able to reproduce it on your machine? What is the asm before/after? |
public enum IntEnum { }
public class Foo
{
[MethodImpl(MethodImplOptions.NoInlining)]
public static bool A((byte, IntEnum, int) a, (byte, IntEnum, int) b)
{
return EqualityComparer<(byte, IntEnum, int)>.Default.Equals(a, b);
}
} New codegen: ; Assembly listing for method Foo:A(System.ValueTuple`3[ubyte,int,int],System.ValueTuple`3[ubyte,int,int]):bool (Tier1)
G_M44446_IG01: ;; offset=0000H
push rdi
push rsi
push rbx
sub rsp, 32
mov rbx, rdx
;; size=10 bbWeight=1 PerfScore 3.50
G_M44446_IG02: ;; offset=000AH
mov esi, dword ptr [rcx]
mov edi, dword ptr [rcx+04H]
movzx rdx, byte ptr [rcx+08H]
movzx r8, byte ptr [rbx+08H]
mov rcx, 0x209F1C01D80 ; const ptr
mov rcx, gword ptr [rcx]
call [System.Collections.Generic.GenericEqualityComparer`1[ubyte]:Equals(ubyte,ubyte):bool:this]
test eax, eax
je SHORT G_M44446_IG06
;; size=37 bbWeight=1 PerfScore 14.50
G_M44446_IG03: ;; offset=002FH
cmp esi, dword ptr [rbx]
jne SHORT G_M44446_IG06
mov r8d, dword ptr [rbx+04H]
mov rcx, 0x209F1C01D90 ; const ptr
mov rcx, gword ptr [rcx]
mov edx, edi
call [System.Collections.Generic.GenericEqualityComparer`1[int]:Equals(int,int):bool:this]
;; size=29 bbWeight=0.50 PerfScore 5.75
G_M44446_IG04: ;; offset=004CH
nop
;; size=1 bbWeight=1 PerfScore 0.25
G_M44446_IG05: ;; offset=004DH
add rsp, 32
pop rbx
pop rsi
pop rdi
ret
;; size=8 bbWeight=1 PerfScore 2.75
G_M44446_IG06: ;; offset=0055H
xor eax, eax
jmp SHORT G_M44446_IG04
;; size=4 bbWeight=0 PerfScore 0.00
; Total bytes of code 89 Perhaps, the specialized byte comparer had more inline-friendly Equals.. |
|
Before: ; Devirtualization.EqualityComparerFixture`1[[System.ValueTuple`3[[System.Byte, System.Private.CoreLib],[Devirtualization.EqualityComparer+E, MicroBenchmarks],[System.Int32, System.Private.CoreLib]], System.Private.CoreLib]].Compare(System.ValueTuple`3<Byte,E,Int32> ByRef, System.ValueTuple`3<Byte,E,Int32> ByRef)
sub rsp,28
mov ecx,[rdx]
mov eax,[rdx+4]
movzx edx,byte ptr [rdx+8]
mov r10d,[r8]
mov r9d,[r8+4]
movzx r8d,byte ptr [r8+8]
cmp dl,r8b
jne short M01_L01
cmp ecx,r10d
jne short M01_L01
mov rcx,0C7C0001E30
mov rcx,[rcx]
mov edx,eax
mov r8d,r9d
call qword ptr [7FFA767517E8]; System.Collections.Generic.GenericEqualityComparer`1[[System.Int32, System.Private.CoreLib]].Equals(Int32, Int32)
M01_L00:
nop
add rsp,28
ret
M01_L01:
xor eax,eax
jmp short M01_L00
; Total bytes of code 69 After: ; Devirtualization.EqualityComparerFixture`1[[System.ValueTuple`3[[System.Byte, System.Private.CoreLib],[Devirtualization.EqualityComparer+E, MicroBenchmarks],[System.Int32, System.Private.CoreLib]], System.Private.CoreLib]].Compare(System.ValueTuple`3<Byte,E,Int32> ByRef, System.ValueTuple`3<Byte,E,Int32> ByRef)
push rdi
push rsi
push rbp
push rbx
sub rsp,28
mov ebx,[rdx]
mov esi,[rdx+4]
movzx edx,byte ptr [rdx+8]
mov edi,[r8]
mov ebp,[r8+4]
movzx r8d,byte ptr [r8+8]
mov rcx,0F521001F00
mov rcx,[rcx]
call qword ptr [7FFA76B8FDC0]; System.Collections.Generic.GenericEqualityComparer`1[[System.Byte, System.Private.CoreLib]].Equals(Byte, Byte)
test eax,eax
je short M01_L01
cmp ebx,edi
jne short M01_L01
mov rcx,0F521001E30
mov rcx,[rcx]
mov edx,esi
mov r8d,ebp
call qword ptr [7FFA76761B10]; System.Collections.Generic.GenericEqualityComparer`1[[System.Int32, System.Private.CoreLib]].Equals(Int32, Int32)
M01_L00:
nop
add rsp,28
pop rbx
pop rbp
pop rsi
pop rdi
ret
M01_L01:
xor eax,eax
jmp short M01_L00
; Total bytes of code 94 |
Should I restore
I think that this recursion check should be changed to only block on the same generic instantiation. |
Yes, this is caused by a variation of #58824: the IL is the same but the method is a different instantiation. I will look into fixing that. It may not be very surgical though. |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsRun Information
Regressions in Devirtualization.EqualityComparer
ReproGeneral Docs link: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net8.0 --filter 'Devirtualization.EqualityComparer*' PayloadsDevirtualization.EqualityComparer.ValueTupleCompareETL FilesHistogram
Description of detection logic
JIT DisasmsDevirtualization.EqualityComparer.ValueTupleCompareNoOptETL FilesHistogram
Description of detection logic
JIT DisasmsDevirtualization.EqualityComparer.ValueTupleCompareCachedETL FilesHistogram
Description of detection logic
JIT DisasmsDevirtualization.EqualityComparer.ValueTupleCompareWrappedETL FilesHistogram
Description of detection logic
JIT DisasmsDocsProfiling workflow for dotnet/runtime repository
|
The existing check was too conservative, and blocked inlines of one instantation of a generic method into a different instantiation of the same method, or of two different methods that share the exact same IL stream. Generalize the check to also compare the method handle and runtime context. Fixes dotnet#88667 Fixes dotnet#58824
The existing check was too conservative, and blocked inlines of one instantation of a generic method into a different instantiation of the same method, or of two different methods that share the exact same IL stream. Generalize the check to also compare the method handle and runtime context. Fixes #88667 Fixes #58824
Run Information
Regressions in Devirtualization.EqualityComparer
Test Report
Repro
General Docs link: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md
Payloads
Baseline
Compare
Devirtualization.EqualityComparer.ValueTupleCompare
ETL Files
Histogram
Description of detection logic
JIT Disasms
Devirtualization.EqualityComparer.ValueTupleCompareNoOpt
ETL Files
Histogram
Description of detection logic
JIT Disasms
Devirtualization.EqualityComparer.ValueTupleCompareCached
ETL Files
Histogram
Description of detection logic
JIT Disasms
Devirtualization.EqualityComparer.ValueTupleCompareWrapped
ETL Files
Histogram
Description of detection logic
JIT Disasms
Docs
Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
The text was updated successfully, but these errors were encountered: