Skip to content

JIT: Skip switch recognition for handles#104634

Merged
jakobbotsch merged 1 commit intodotnet:mainfrom
jakobbotsch:skip-switch-recognition-handles
Jul 10, 2024
Merged

JIT: Skip switch recognition for handles#104634
jakobbotsch merged 1 commit intodotnet:mainfrom
jakobbotsch:skip-switch-recognition-handles

Conversation

@jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jul 9, 2024

Recognizing a series of handle compares as a switch is not legal since those handles may change values.

I hit this issue in #104603 where the switch recognition managed to turn the following assert into a switch:

runtime/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/ReflectionCoreCallbacksImplementation.cs

Line 415 in 7f5ce1e

Debug.Assert(x is byte or ushort or uint or ulong);

Baseline codegen was:

G_M9579_IG02:  ;; offset=0x0012
       test     rbx, rbx
       je       SHORT G_M9579_IG04
						;; size=5 bbWeight=1 PerfScore 1.25
G_M9579_IG03:  ;; offset=0x0017
       mov      rdi, qword ptr [rbx]
       lea      rcx, [(reloc 0x4000000000423520)]      ; System.Byte
       cmp      rdi, rcx
       je       G_M9579_IG10
       lea      rcx, [(reloc 0x4000000000423530)]      ; System.UInt16
       cmp      rdi, rcx
       je       G_M9579_IG10
       lea      rcx, [(reloc 0x4000000000423538)]      ; System.UInt32
       cmp      rdi, rcx
       je       SHORT G_M9579_IG10
       lea      rcx, [(reloc 0x4000000000423540)]      ; System.UInt64
       cmp      rdi, rcx
       je       SHORT G_M9579_IG10
						;; size=59 bbWeight=0.25 PerfScore 2.25
G_M9579_IG04:  ;; offset=0x0052
       lea      rcx, gword ptr [(reloc 0x40000000004206d8)]      ; '""'
       lea      rdx, gword ptr [(reloc 0x40000000004206d8)]      ; '""'
       call     System.Diagnostics.Debug:Fail(System.String,System.String)
       test     rbx, rbx
       jne      SHORT G_M9579_IG10

With the switch recognition, we were ending up with:

G_M9579_IG02:  ;; offset=0x0012
       test     rbx, rbx
       je       SHORT G_M9579_IG05
						;; size=5 bbWeight=1 PerfScore 1.25
G_M9579_IG03:  ;; offset=0x0017
       mov      rdi, qword ptr [rbx]
       lea      rcx, [(reloc 0x4000000000423578)]      ; System.Byte
       cmp      rdi, rcx
       je       G_M9579_IG11
       mov      rcx, 0x4000000000423588
       sub      rdi, rcx
       cmp      rdi, 16
       ja       SHORT G_M9579_IG05
						;; size=38 bbWeight=0.25 PerfScore 1.38
G_M9579_IG04:  ;; offset=0x003D
       mov      eax, 0x10101
       bt       eax, edi
       jb       SHORT G_M9579_IG11
						;; size=10 bbWeight=0.25 PerfScore 0.43

This PR puts us back at the baseline.

@jakobbotsch
JIT: Skip switch recognition for handles
2191c81 
Recognizing a series of handle compares as a switch is not legal since
those handles may change values.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 9, 2024
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jul 9, 2024

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

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jul 9, 2024

cc @dotnet/jit-contrib PTAL @EgorBo

@jakobbotsch jakobbotsch requested a review from EgorBo July 9, 2024 19:38
@jakobbotsch jakobbotsch mentioned this pull request Jul 9, 2024
Open
EgorBo
EgorBo approved these changes Jul 9, 2024
View reviewed changes
Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ouch, good catch!

@jakobbotsch jakobbotsch mentioned this pull request Jul 9, 2024
Merged
@build-analysis build-analysis bot mentioned this pull request Jul 9, 2024
Closed
@jakobbotsch jakobbotsch merged commit cd9cc87 into dotnet:main Jul 10, 2024
@jakobbotsch jakobbotsch deleted the skip-switch-recognition-handles branch July 10, 2024 06:58
matouskozak added a commit to matouskozak/runtime that referenced this pull request Jul 11, 2024
@matouskozak
Revert "JIT: Skip switch recognition for handles (dotnet#104634)"
572014a 
This reverts commit cd9cc87.
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBo EgorBo EgorBo approved these changes

Assignees

@jakobbotsch jakobbotsch

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jakobbotsch @EgorBo