Skip to content
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

JIT: optRedundantBranches misses an opportunity #70480

Closed
EgorBo opened this issue Jun 9, 2022 · 4 comments · Fixed by #70907
Closed

JIT: optRedundantBranches misses an opportunity #70480

EgorBo opened this issue Jun 9, 2022 · 4 comments · Fixed by #70907
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Jun 9, 2022

Extracted & simplified from #36649 (comment) to a separate issue

void PrintN(object o)
{
    if (o is int n)
        Console.WriteLine(n);
}

This simple method emits a lot of codegen:

; Method testout1:PrintN(System.Object):this
G_M58869_IG01:              ;; offset=0000H
       56                   push     rsi
       4883EC20             sub      rsp, 32
       488BF2               mov      rsi, rdx
G_M58869_IG02:              ;; offset=0008H
       488BD6               mov      rdx, rsi
       4885D2               test     rdx, rdx
       7442                 je       SHORT G_M58869_IG08
G_M58869_IG03:              ;; offset=0010H
       488B12               mov      rdx, qword ptr [rdx]
       48B9B86A32F4FA7F0000 mov      rcx, 0x7FFAF4326AB8      ; System.Int32
       483BD1               cmp      rdx, rcx
       7530                 jne      SHORT G_M58869_IG08
G_M58869_IG04:              ;; offset=0022H
       48B9B86A32F4FA7F0000 mov      rcx, 0x7FFAF4326AB8      ; System.Int32
       483BD1               cmp      rdx, rcx
       7413                 je       SHORT G_M58869_IG06
G_M58869_IG05:              ;; offset=0031H
       488BD6               mov      rdx, rsi
       48B9B86A32F4FA7F0000 mov      rcx, 0x7FFAF4326AB8      ; System.Int32
       FF15FC2BFDFF         call     [CORINFO_HELP_UNBOX]
G_M58869_IG06:              ;; offset=0044H
       8B4E08               mov      ecx, dword ptr [rsi+8]
G_M58869_IG07:              ;; offset=0047H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       FF2586E73900         tail.jmp [System.Console:WriteLine(int)]
G_M58869_IG08:              ;; offset=0052H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       C3                   ret      
; Total bytes of code: 88

image

From this flowgraph (built after "Redundant branch opts" phase) it's obvious that BB06 is dominated by BB02 with exactly the same condition which means BB06 can be folded (and BB07 becomes dead as well).

A not very TP-wise solution is basically run a sort of "UpdateFlowGraph" phase to drop unreachable blocks and recompute doms and then run optRedundantBranches again

cc @AndyAyersMS

@EgorBo EgorBo added the tenet-performance Performance related issue label Jun 9, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 9, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 9, 2022
@ghost
Copy link

ghost commented Jun 9, 2022

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

Issue Details

Extracted & simplified from #36649 (comment) to a separate issue

void PrintN(object o)
{
    if (o is int n)
        Console.WriteLine(n);
}

This simple method emits a lot of codegen:

; Method testout1:PrintN(System.Object):this
G_M58869_IG01:              ;; offset=0000H
       56                   push     rsi
       4883EC20             sub      rsp, 32
       488BF2               mov      rsi, rdx
G_M58869_IG02:              ;; offset=0008H
       488BD6               mov      rdx, rsi
       4885D2               test     rdx, rdx
       7442                 je       SHORT G_M58869_IG08
G_M58869_IG03:              ;; offset=0010H
       488B12               mov      rdx, qword ptr [rdx]
       48B9B86A32F4FA7F0000 mov      rcx, 0x7FFAF4326AB8      ; System.Int32
       483BD1               cmp      rdx, rcx
       7530                 jne      SHORT G_M58869_IG08
G_M58869_IG04:              ;; offset=0022H
       48B9B86A32F4FA7F0000 mov      rcx, 0x7FFAF4326AB8      ; System.Int32
       483BD1               cmp      rdx, rcx
       7413                 je       SHORT G_M58869_IG06
G_M58869_IG05:              ;; offset=0031H
       488BD6               mov      rdx, rsi
       48B9B86A32F4FA7F0000 mov      rcx, 0x7FFAF4326AB8      ; System.Int32
       FF15FC2BFDFF         call     [CORINFO_HELP_UNBOX]
G_M58869_IG06:              ;; offset=0044H
       8B4E08               mov      ecx, dword ptr [rsi+8]
G_M58869_IG07:              ;; offset=0047H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       FF2586E73900         tail.jmp [System.Console:WriteLine(int)]
G_M58869_IG08:              ;; offset=0052H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       C3                   ret      
; Total bytes of code: 88

image

From this flowgraph (built after "Redundant branch opts" phase) it's obvious that BB06 is dominated by BB02 with exactly the same condition which means BB06 can be folded.

A not very TP-wise solution is basically run a sort of "UpdateFlowGraph" phase to drop unreachable blocks and recompute doms and then run optRedundantBranches again

cc @AndyAyersMS

Author: EgorBo
Assignees: -
Labels:

tenet-performance, area-CodeGen-coreclr

Milestone: -

@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Jun 9, 2022
@EgorBo EgorBo added this to the 8.0.0 milestone Jun 9, 2022
@Wraith2
Copy link
Contributor

Wraith2 commented Jun 9, 2022

Isn't IG04 reachable from exiting IG03 with an equality on the comparison?

@EgorBo
Copy link
Member Author

EgorBo commented Jun 9, 2022

Isn't IG04 reachable from exiting IG03 with an equality on the comparison?

BB03 is BBJ_ALWAYS to BB09, so no

@AndyAyersMS
Copy link
Member

There is a bit of precedent in the JIT for updating dominator tree links as we optimize, so it's possible that we could look into something like that. But even with this, RBO might not visit the branches in the right order, so might need the ability to revisit cases that were already looked at...

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 17, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants