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

[NativeAot] Devirtualization opportunity around field loads #74000

Closed
MichalStrehovsky opened this issue Aug 16, 2022 · 6 comments · Fixed by #74075
Closed

[NativeAot] Devirtualization opportunity around field loads #74000

MichalStrehovsky opened this issue Aug 16, 2022 · 6 comments · Fixed by #74075
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@MichalStrehovsky
Copy link
Member

Notice the first interface call is not devirtualized, only the second, after storing to a local.

using System;
using System.Runtime.CompilerServices;

interface IFoo
{
    void Frob();
}

class Program : IFoo
{
    static IFoo theFoo = new Program();

    public void Frob() => Console.WriteLine("Hello");

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Main()
    {
        theFoo.Frob();
        IFoo i = theFoo;
        i.Frob();
    }
}
00007FF796607C10 56                   push        rsi  
00007FF796607C11 48 83 EC 20          sub         rsp,20h  
00007FF796607C15 E8 8B A8 D4 FF       call        __GetGCStaticBase_repro_Program (07FF7963524A5h)  
00007FF796607C1A 48 8B F0             mov         rsi,rax  
00007FF796607C1D 48 8B 4E 08          mov         rcx,qword ptr [rsi+8]  
00007FF796607C21 4C 8D 15 A8 EC 27 00 lea         r10,[__InterfaceDispatchCell_repro_IFoo__Frob_repro_Program__Main (07FF7968868D0h)]  
00007FF796607C28 41 FF 12             call        qword ptr [r10]  
00007FF796607C2B 48 8B 4E 08          mov         rcx,qword ptr [rsi+8]  
00007FF796607C2F 38 09                cmp         byte ptr [rcx],cl  
00007FF796607C31 48 8D 0D 30 47 26 00 lea         rcx,[__Str_Hello (07FF79686C368h)]  
00007FF796607C38 E8 13 26 00 00       call        System_Console_System_Console__WriteLine_12 (07FF79660A250h)  
00007FF796607C3D 90                   nop  
00007FF796607C3E 48 83 C4 20          add         rsp,20h  
00007FF796607C42 5E                   pop         rsi  
00007FF796607C43 C3                   ret  

Cc @EgorBo

@MichalStrehovsky MichalStrehovsky added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 16, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 16, 2022
@ghost
Copy link

ghost commented Aug 16, 2022

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

Issue Details

Notice the first interface call is not devirtualized, only the second, after storing to a local.

using System;
using System.Runtime.CompilerServices;

interface IFoo
{
    void Frob();
}

class Program : IFoo
{
    static IFoo theFoo = new Program();

    public void Frob() => Console.WriteLine("Hello");

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Main()
    {
        theFoo.Frob();
        IFoo i = theFoo;
        i.Frob();
    }
}
00007FF796607C10 56                   push        rsi  
00007FF796607C11 48 83 EC 20          sub         rsp,20h  
00007FF796607C15 E8 8B A8 D4 FF       call        __GetGCStaticBase_repro_Program (07FF7963524A5h)  
00007FF796607C1A 48 8B F0             mov         rsi,rax  
00007FF796607C1D 48 8B 4E 08          mov         rcx,qword ptr [rsi+8]  
00007FF796607C21 4C 8D 15 A8 EC 27 00 lea         r10,[__InterfaceDispatchCell_repro_IFoo__Frob_repro_Program__Main (07FF7968868D0h)]  
00007FF796607C28 41 FF 12             call        qword ptr [r10]  
00007FF796607C2B 48 8B 4E 08          mov         rcx,qword ptr [rsi+8]  
00007FF796607C2F 38 09                cmp         byte ptr [rcx],cl  
00007FF796607C31 48 8D 0D 30 47 26 00 lea         rcx,[__Str_Hello (07FF79686C368h)]  
00007FF796607C38 E8 13 26 00 00       call        System_Console_System_Console__WriteLine_12 (07FF79660A250h)  
00007FF796607C3D 90                   nop  
00007FF796607C3E 48 83 C4 20          add         rsp,20h  
00007FF796607C42 5E                   pop         rsi  
00007FF796607C43 C3                   ret  

Cc @EgorBo

Author: MichalStrehovsky
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.0 milestone Aug 16, 2022
@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Aug 16, 2022
@EgorBo
Copy link
Member

EgorBo commented Aug 17, 2022

@MichalStrehovsky is there any known build issues for NativeAOT in Main - for some reason I can't run ilc from VS, it throws
image

PS: ah, works from command line

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 17, 2022
@MichalStrehovsky
Copy link
Member Author

@MichalStrehovsky is there any known build issues for NativeAOT in Main - for some reason I can't run ilc from VS, it throws

Weird, I see this too.

I narrowed this down to #73987. If I comment out the <PublishSingleFile> line debugging works again.

@dotnet/dotnet-diag is it known that <PublishSingleFile> in the project file would break debugging in VS? We're using Preview 7 in the repo.

The repro steps are:

  1. Build at least the clr.aot subset of the repo (e.g. build.cmd clr.aot -rc Debug -lc Release, but if you build all of clr, that's fine too)
  2. Open the src\coreclr\tools\aot\ilc.sln solution in VS.
  3. Choose the configuration you built (Debug in my case)
  4. Hit F5 to run under debugger.

@noahfalk
Copy link
Member

@mikem8361 should probably take a look

Mike and Juan did work earlier in this release to make single file debugging work so no, I wouldn't expect it to be failing here.

@MichalStrehovsky
Copy link
Member Author

I'm submitting a workaround for the debugging issue in #74138.

@hoyosjs
Copy link
Member

hoyosjs commented Aug 18, 2022

VS merged support for it - I'm not sure if it's made it's way to public previews yet.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Oct 5, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 5, 2022
@EgorBo EgorBo self-assigned this Jan 2, 2023
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 a pull request may close this issue.

5 participants