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

Compute precise generic context after de-virtualization #38477

Closed
jkotas opened this issue Jun 26, 2020 · 4 comments · Fixed by #45526
Closed

Compute precise generic context after de-virtualization #38477

jkotas opened this issue Jun 26, 2020 · 4 comments · Fixed by #45526
Assignees
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

@jkotas
Copy link
Member

jkotas commented Jun 26, 2020

Compile with optimizations on:

using System;
using System.Runtime.CompilerServices;

class G<T>
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public virtual bool M() => typeof(T) == typeof(string);
}

class Program
{
    static int Main() => new G<string>().M() ? 1 : -1;
}

Expected result (disassembly of Main method):

     mov      eax, 1
     ret

Actual result:

G_M24375_IG01:
       4883EC28             sub      rsp, 40
                                                ;; bbWeight=1    PerfScore 0.25
G_M24375_IG02:
       48B9F0A37960FD7F0000 mov      rcx, 0x7FFD6079A3F0
       E8CDF1905F           call     CORINFO_HELP_NEWSFAST
       488BC8               mov      rcx, rax
        // The call is devirtualized properly, but inlining fails due to imprecise generic context
        // Note that the inlining succeeds when M is changed to be non-virtual
       FF15F4672700         call     [G`1[__Canon][System.__Canon]:M():bool:this]
       85C0                 test     eax, eax
       750A                 jne      SHORT G_M24375_IG05
                                                ;; bbWeight=1    PerfScore 5.75
G_M24375_IG03:
       B8FFFFFFFF           mov      eax, -1
                                                ;; bbWeight=0.50 PerfScore 0.13
G_M24375_IG04:
       4883C428             add      rsp, 40
       C3                   ret
                                                ;; bbWeight=0.50 PerfScore 0.63
G_M24375_IG05:
       B801000000           mov      eax, 1
                                                ;; bbWeight=0.50 PerfScore 0.13
G_M24375_IG06:
       4883C428             add      rsp, 40
       C3                   ret
                                                ;; bbWeight=0.50 PerfScore 0.63

category:cq
theme:devirtualization
skill-level:expert
cost:medium

@jkotas jkotas added tenet-performance Performance related issue area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Jun 26, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jun 26, 2020
@jkotas
Copy link
Member Author

jkotas commented Jun 26, 2020

Context #38229 (comment)

@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Jun 27, 2020
@JulieLeeMSFT JulieLeeMSFT added this to the Future milestone Jun 27, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@AndyAyersMS
Copy link
Member

Think we should fix this in .NET 6, as part of removing barriers to efficient generic composition.

@AndyAyersMS AndyAyersMS modified the milestones: Future, 6.0.0 Nov 2, 2020
@AndyAyersMS AndyAyersMS added this to Needs Triage in .NET Core CodeGen via automation Nov 2, 2020
@AndyAyersMS AndyAyersMS moved this from Needs Triage to Backlog (General) in .NET Core CodeGen Nov 2, 2020
@Rattenkrieg
Copy link
Contributor

I did some research while debugging #43668 I will share my findings (and maybe I will be able to craft some PoC) after I'll done with #43668

@AndyAyersMS AndyAyersMS removed the JitUntriaged CLR JIT issues needing additional triage label Nov 3, 2020
@AndyAyersMS AndyAyersMS self-assigned this Nov 3, 2020
@AndyAyersMS
Copy link
Member

Is this just a matter of updating the context (and the exact context) to either the method context (like we do now) or the object class context...?

Seems to work on this simple example anyways, though the codegen creates a frame it doesn't use:

; Assembly listing for method Program:Main():int
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 OutArgs      [V00    ] (  1,  1   )  lclBlk (32) [rsp+0x00]   "OutgoingArgSpace"
;* V01 tmp1         [V01,T00] (  0,  0   )     ref  ->  zero-ref    class-hnd exact "NewObj constructor temp"
;
; Lcl frame size = 40

G_M24375_IG01:              ;; offset=0000H
       4883EC28             sub      rsp, 40
                                                ;; bbWeight=1    PerfScore 0.25
G_M24375_IG02:              ;; offset=0004H
       B801000000           mov      eax, 1
                                                ;; bbWeight=1    PerfScore 0.25
G_M24375_IG03:              ;; offset=0009H
       4883C428             add      rsp, 40
       C3                   ret
                                                ;; bbWeight=1    PerfScore 1.25
Inlines into 06000003 [via DefaultPolicy] Program:Main():int
  [1 IL=0000 TR=000005 06000002] [below ALWAYS_INLINE size] G`1[__Canon][System.__Canon]:.ctor():this
    [2 IL=0001 TR=000016 0600049E] [below ALWAYS_INLINE size] System.Object:.ctor():this
  [3 IL=0005 TR=000007 06000001] [aggressive inline attribute devirt] G`1[__Canon][System.__Canon]:M():bool:this

This is currently keying off what kind of context gets passed in, so a method context leads to an updated method context, and likewise for class contexts. I wonder if this is sufficiently general.

AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Dec 3, 2020
If we devirtualize to a method on a generic class, try and obtain the
exact class. Pass this back to the jit to unblock some types of inlines.

Also refactor how information is passed during devirtualization in
anticipation of follow on work to devirtualize default interface methods.
Because there are now multiple inputs and outputs, convey everthing
using a struct.

Resolves dotnet#38477.
.NET Core CodeGen automation moved this from Backlog (General) to Done Dec 5, 2020
AndyAyersMS added a commit that referenced this issue Dec 5, 2020
If we devirtualize to a method on a generic class, try and obtain the
exact class. Pass this back to the jit to unblock some types of inlines.

Also refactor how information is passed during devirtualization in
anticipation of follow on work to devirtualize default interface methods.
Because there are now multiple inputs and outputs, convey everything
using a struct.

Resolves #38477.
@ghost ghost locked as resolved and limited conversation to collaborators Jan 4, 2021
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 tenet-performance Performance related issue
Projects
Archived in project
6 participants