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: missing conditional branch merging across multiple BBs #59863

Open
Sergio0694 opened this issue Oct 1, 2021 · 2 comments
Open

JIT: missing conditional branch merging across multiple BBs #59863

Sergio0694 opened this issue Oct 1, 2021 · 2 comments
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

@Sergio0694
Copy link
Contributor

Sergio0694 commented Oct 1, 2021

Description

RyuJit currently misses some optimization opportunities when there are multiple BBs with conditional branches that could be merged, and instead result in avoidable conditional branches in the final codegen. Consider this example:

static unsafe bool Test(int* a, int* b)
{
    return a < b || a == b;
}

This results in the following codegen:

L0000: cmp ecx, edx
L0002: jb short L000d
L0004: cmp ecx, edx
L0006: sete al
L0009: movzx eax, al
L000c: ret
L000d: mov eax, 1
L0012: ret

Here the two BBs with the LT and EQ tests could just be combined in a single LE one, which could result in eg. a single seta in the final codegen here, or in a single jle if the method was inlined and the caller was to do a conditional branch on its return.

Courtesy of @EgorBo for these trees, we basically need the following optimization:

*  JTRUE   
\--*  LT  
   +--*  LCL_VAR  arg0         
    \--*  LCL_VAR  arg1         

*  JTRUE or RETURN   
\--*  EQ           
   +--*  LCL_VAR  arg0         
    \--*  LCL_VAR  arg1

to

*  JTRUE   
\--*  LE  
   +--*  LCL_VAR  arg0         
    \--*  LCL_VAR  arg1

And the same for all possible conditional combinations as well (eg. GT+EQ to GE, etc.).

Configuration

Tested on .NET 5 stable, and also on .NET 6 (daily builds, with Disasmo).

Regression?

Nope.

category:correctness
theme:ir
skill-level:intermediate
cost:small
impact:small

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

ghost commented Oct 1, 2021

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

Issue Details

Description

RyuJit currently misses some optimization opportunities when there are multiple BBs with conditional branches that could be merged, and instead result in avoidable conditional branches in the final codegen. Consider this example:

static unsafe bool Test(int* a, int* b)
{
    return a < b || a == b;
}

This results in the following codegen:

L0000: cmp ecx, edx
L0002: jb short L000d
L0004: cmp ecx, edx
L0006: sete al
L0009: movzx eax, al
L000c: ret
L000d: mov eax, 1
L0012: ret

Here the two BBs with the LT and EQ tests could just be combined in a single LE one, which could result in eg. a single seta in the final codegen here, or in a single jle if the method was inlined and the caller was to do a conditional branch on its return.

Courtesy of @EgorBo for these trees, we basically need the following optimization:

*  JTRUE   
\--*  LT  
   +--*  LCL_VAR  arg0         
    \--*  LCL_VAR  arg1         

*  JTRUE or RETURN   
\--*  EQ           
   +--*  LCL_VAR  arg0         
    \--*  LCL_VAR  arg1

to

*  JTRUE   
\--*  LE  
   +--*  LCL_VAR  arg0         
    \--*  LCL_VAR  arg1

And the same for all possible conditional combinations as well (eg. GT+EQ to GE, etc.).

Configuration

Tested on .NET 5 stable, and also on .NET 6 (daily builds, with Disasmo).

Regression?

Nope.

Author: Sergio0694
Assignees: -
Labels:

tenet-performance, area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Oct 1, 2021

From my understanding it can be done in optOptimizeBools without introducing additional loops over BBs.

@EgorBo EgorBo added this to the Future milestone Oct 1, 2021
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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

No branches or pull requests

2 participants