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

Propagate typeof() during inlining #71778

Merged
merged 8 commits into from
Jul 10, 2022
Merged

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Jul 7, 2022

Closes #40381

Example:

static void Main(string[] args)
{
    Console.WriteLine(Foo(typeof(float)));
}

static bool Foo(Type t)
{
    if (t == typeof(int))
        Console.WriteLine("1");
    if (t == typeof(uint))
        Console.WriteLine("2");
    if (t == typeof(long))
        Console.WriteLine("3");
    if (t == typeof(ulong))
        Console.WriteLine("4");
    return t.IsValueType;
}

Codegen for Main():

       4883EC28             sub      rsp, 40
						;; size=4 bbWeight=1    PerfScore 0.25
G_M60419_IG02:              ;; offset=0004H
       B901000000           mov      ecx, 1
       FF15B1F43800         call     [System.Console:WriteLine(bool)]
       90                   nop      
						;; size=12 bbWeight=1    PerfScore 3.50
G_M60419_IG03:              ;; offset=0010H
       4883C428             add      rsp, 40
       C3                   ret  

It now:

  1. Inliner now understands that typeof() at that call-site is a constant arg -> recognizes 4 foldabale branches in this inlinee:
Inline candidate has 4 foldable branches.  Multiplier increased to 8.
  1. Propagates typeof() by value during inlining - it helps to eliminate those branches and fix RyuJIT: IsValueType and IsAssignableFrom optimizations aren't inlining friendly. #40381

Regressions are due to inlining, RA and some cases where CSE/Hoisting doesn't "undo" propagation, e.g.: https://www.diffchecker.com/ukNlUweZ

Also a lot of regressions coming from the fact that we never do CSE across blocks, e.g.:

static void Bar(int a) => Foo(a, typeof(int));

static void Foo(int a, Type t)
{
    if (a == 42)
        Console.WriteLine(t);
    else
        Console.WriteLine(t);
}

codegen diff for Bar: https://www.diffchecker.com/mWkBicM0
but it's not really a terrible perf regression I'd guess...

@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 Jul 7, 2022
@ghost ghost assigned EgorBo Jul 7, 2022
@ghost
Copy link

ghost commented Jul 7, 2022

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

Issue Details

Quick experiment, locally saw more improvements than regressions from it

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo EgorBo marked this pull request as draft July 7, 2022 18:43
@EgorBo EgorBo marked this pull request as ready for review July 8, 2022 23:29
@EgorBo
Copy link
Member Author

EgorBo commented Jul 9, 2022

PTAL @dotnet/jit-contrib @AndyAyersMS
overall, good diffs. Size regressions mostly due to the fact we propagate typeof() into different basic blocks and then don't CSE it back - but it might be a perf improvement actually, because we don't call the helper when it's not needed.

EgorBo and others added 3 commits July 9, 2022 21:14
Co-authored-by: Andy Ayers <andya@microsoft.com>
@EgorBo
Copy link
Member Author

EgorBo commented Jul 10, 2022

@AndyAyersMS anything else?

@EgorBo EgorBo merged commit 3052905 into dotnet:main Jul 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 9, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RyuJIT: IsValueType and IsAssignableFrom optimizations aren't inlining friendly.
2 participants