JIT: Expand GT_RETURN condition to GT_JTRUE

EgorBo commented 2 years ago

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

ghost commented 2 years ago

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 ```csharp return n == 42; ``` to ```csharp 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.: ```csharp 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`: ```asm ; 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: ```asm 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 commented 5 days ago

dotnet / runtime