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: Expand GT_RETURN condition to GT_JTRUE #65370

Open
EgorBo opened this issue Feb 15, 2022 · 2 comments
Open

JIT: Expand GT_RETURN condition to GT_JTRUE #65370

EgorBo opened this issue Feb 15, 2022 · 2 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Feb 15, 2022

1. Import return cond; as return cond ? true : false

Expand expressions like

return n == 42;

to

if (n == 42)
    return true;
return false;

Early in the importer/morph so we can then properly merge returns, apply branch-to-cmov (or optOptimizeBool) kinds of optimizations. It's a sort of canonization, we then should convert it back to return cmp or even optimize to cmov/csel if necessary. It will allow us to forget about various ? true : false hacks in the codebase.

2. Duplicate returns for return GT_RET_EXPR

currently, when we inline a method under GT_RETURN node, e.g.:

static int Foo(int x)
{
    return Bar(x);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static int Bar(int x)
{
    if (x == 42)
        return 10;
    if (x <= 0)
        return 0;
    return 100;
}

We end up with 4 jmp instructions (while 2 would be enough) in the codegen of Foo:

; Assembly listing for method Program:Foo(int):int
       cmp      ecx, 42
       jne      SHORT G_M29289_IG04
       mov      eax, 10
       jmp      SHORT G_M29289_IG06
G_M29289_IG04:
       test     ecx, ecx
       jg       SHORT G_M29289_IG05
       xor      eax, eax
       jmp      SHORT G_M29289_IG06
G_M29289_IG05:
       mov      eax, 100
G_M29289_IG06:
       ret 

because we end up spilling Bar into a temp during inlining while we should just insert it as is and remove parent GT_RETURN
Expected codegen:

; Assembly listing for method Program:Foo(int):int
       cmp      ecx, 42
       jne      SHORT G_M8222_IG05
       mov      eax, 10
       ret      
G_M8222_IG05:
       test     ecx, ecx
       jg       SHORT G_M8222_IG07
       xor      eax, eax
       ret      
G_M8222_IG07:
       mov      eax, 100
       ret

just two jumps. If epilogues are big or too many of them a separate phase should merge them.

category:implementation
theme:ir

@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 Feb 15, 2022
@ghost
Copy link

ghost commented Feb 15, 2022

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

Issue Details

1. return cond; to return cond ? true : false

Expand expressions like

return n == 42;

to

if (n == 42)
    return true;
return false;

Early in the importer/morph so we can then properly merge returns, apply branch-to-cmov (or optOptimizeBool) kinds of optimizations. It's a sort of canonization, we then should convert it back to return cmp or even optimize to cmov/csel if necessary then. It will allow us to forget about various ? true : false hacks in the codebase.

2. Duplicate returns for return GT_RET_EXPR

currently, when we inline a method under GT_RETURN node, e.g.:

static int Foo(int x)
{
    return Bar(x);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static int Bar(int x)
{
    if (x == 42)
        return 10;
    if (x <= 0)
        return 0;
    return 100;
}

We end up with 4 jmp instructions (while 2 would be enough) in the codegen of Foo:

; Assembly listing for method Program:Bar(int):int
G_M8222_IG01:
G_M8222_IG02:
       cmp      ecx, 42
       jne      SHORT G_M8222_IG05
G_M8222_IG03:
       mov      eax, 10
G_M8222_IG04:
       ret      
G_M8222_IG05:
       test     ecx, ecx
       jg       SHORT G_M8222_IG07
       xor      eax, eax
G_M8222_IG06:
       ret      
G_M8222_IG07:
       mov      eax, 100
G_M8222_IG08:
       ret      

because we end up spilling Bar into a temp during inlining while we should just insert it as is and remove parent GT_RETURN
Expected codegen:

       cmp      ecx, 42
       jne      SHORT G_M8222_IG05
       mov      eax, 10
       ret      
G_M8222_IG05:
       test     ecx, ecx
       jg       SHORT G_M8222_IG07
       xor      eax, eax
       ret      
G_M8222_IG07:
       mov      eax, 100
       ret

just two jumps. If epilogues are big or too many of them a separate phase should merge them.

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@xtqqczze
Copy link
Contributor

Related: #8363.

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
Projects
Status: Optimizations
Development

No branches or pull requests

2 participants