dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.06k stars 4.69k forks source link

JIT: missing opportunity for Redundant Branch Opt #98227

Open EgorBo opened 7 months ago

EgorBo commented 7 months ago

A simplified repro of what I saw in a real code:

void Test(int a)
{
    if (a >= 100)
    {
        if (a <= 100)
        {
            Console.WriteLine(a);
        }
    }
}

Current codegen:

; Method Prog:Test(int):this (FullOpts)
G_M36082_IG01:
       sub      rsp, 40
G_M36082_IG02:
       cmp      edx, 100
       jl       SHORT G_M36082_IG04
G_M36082_IG03:
       cmp      edx, 100
       jg       SHORT G_M36082_IG04
       mov      ecx, edx
       call     [System.Console:WriteLine(int)]
G_M36082_IG04:
       nop      
G_M36082_IG05:
       add      rsp, 40
       ret      
; Total bytes of code: 28

Expected codegen:

; Method Prog:Test(int):this (FullOpts)
G_M36082_IG01:
       sub      rsp, 40
G_M36082_IG02:
       cmp      edx, 100
       jne      SHORT G_M36082_IG04
G_M36082_IG03:
       mov      ecx, 100
       call     [System.Console:WriteLine(int)]
G_M36082_IG04:
       nop      
G_M36082_IG05:
       add      rsp, 40
       ret      
; Total bytes of code: 26
ghost commented 7 months ago

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

Issue Details
A simplified repro of what I saw in a real code: ```cs void Test(int a) { if (a >= 100) { if (a <= 100) { Console.WriteLine(a); } } } ``` Current codegen: ```asm ; Method Prog:Test(int):this (FullOpts) G_M36082_IG01: sub rsp, 40 G_M36082_IG02: cmp edx, 100 jl SHORT G_M36082_IG04 G_M36082_IG03: cmp edx, 100 jg SHORT G_M36082_IG04 mov ecx, edx call [System.Console:WriteLine(int)] G_M36082_IG04: nop G_M36082_IG05: add rsp, 40 ret ; Total bytes of code: 28 ``` Expected codegen: ```asm ; Method Prog:Test(int):this (FullOpts) G_M36082_IG01: sub rsp, 40 G_M36082_IG02: cmp edx, 100 jne SHORT G_M36082_IG04 G_M36082_IG03: mov ecx, 100 call [System.Console:WriteLine(int)] G_M36082_IG04: nop G_M36082_IG05: add rsp, 40 ret ; Total bytes of code: 26 ```
Author: EgorBo
Assignees: -
Labels: `area-CodeGen-coreclr`, `untriaged`
Milestone: -
EgorBo commented 7 months ago

сс @AndyAyersMS I assume this is slightly different from "implied relops" opts we have. In this case we can: 1) Change the 2nd condition to just == 100 (and then assertprop can propagate it as a constant) 2) Ideally change the dominating comparison if there are no statements between two?

AndyAyersMS commented 7 months ago

Looks similar to the case noted in https://github.com/dotnet/runtime/issues/81220#issuecomment-1477080383.

Linked this example into https://github.com/dotnet/runtime/issues/48115.