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

Final method won't devirt when call-site is "complex" #11711

Open
gfoidl opened this issue Dec 24, 2018 · 2 comments
Open

Final method won't devirt when call-site is "complex" #11711

gfoidl opened this issue Dec 24, 2018 · 2 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@gfoidl
Copy link
Member

gfoidl commented Dec 24, 2018

In the below example the call res[0] = this.Do(src[0]); won't be inlined, whilst res[0] = this.Do('B'); will be inlined.

using System.Linq;
using System.Runtime.CompilerServices;

namespace ConsoleApp1
{
    class Program
    {
        static int Main(string[] args)
        {
            var foo = new Foo();
            var res = Do(foo);

            return res[0];
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        private static char[] Do(Foo foo)
        {
            //return foo.DoWillDevirt();
            return foo.DoWontDevirt();
        }
    }

    public abstract class Base
    {
        private char[] _src = Enumerable.Repeat('B', 10).ToArray();
        private char[] _res = new char[10];

        public char[] DoWontDevirt()
        {
            char[] src = _src;
            char[] res = _res;

            res[0] = this.Do(src[0]);

            return res;
        }

        public char[] DoWillDevirt()
        {
            char[] src = _src;
            char[] res = _res;

            res[0] = this.Do('B');

            return res;
        }

        public abstract char Do(char c);
    }

    public class Foo : Base
    {
        public sealed override char Do(char c) => (char)(c + 1);
    }
}

Excerpt of JIT dump:

impDevirtualizeCall: Trying to devirtualize virtual call:
    class for 'this' is Base (attrib 21000400)
    base method is Base::Do
    devirt to Base::Do -- inexact or not final
               [000021] --C-G-------              *  CALLV ind int    Base.Do
               [000019] ------------ this in rdi  +--*  LCL_VAR   ref    V00 this         
               [000020] ------------ arg1         \--*  LCL_VAR   int    V02 loc1         
    Class not final or exact, and method not final
NOT Marking call [000021] as guarded devirtualization candidate -- disabled by jit config
INLINER: during 'impMarkInlineCandidate' result 'failed this call site' reason 'target not direct' for 'Base:DoWontDevirt():ref:this' calling 'Base:Do(ushort):ushort:this'
INLINER: during 'impMarkInlineCandidate' result 'failed this call site' reason 'target not direct'

For DoWillDevirt:

impDevirtualizeCall: Trying to devirtualize virtual call:
    class for 'this' is Foo (attrib 21000000)
    base method is Base::Do
    devirt to Foo::Do -- final method
               [000022] --C-G-------              *  CALLV ind int    Base.Do
               [000020] ------------ this in rdi  +--*  LCL_VAR   ref    V00 this         
               [000021] ------------ arg1         \--*  CNS_INT   int    66
    final method; can devirtualize

So the difference is that in the former case the JIT won't recognize the current type class for 'this' is Base vs. class for 'this' is Foo.

In the IL there is nothing special for either case:

.method public hidebysig
    instance char[] DoWontDevirt () cil managed
{
    // Method begins at RVA 0x2070
    // Code size 28 (0x1c)
    .maxstack 4
    .locals init (
        [0] char[],
        [1] char
    )

    IL_0000: ldarg.0
    IL_0001: ldfld char[] ConsoleApp1.Base::_src
    IL_0006: ldarg.0
    IL_0007: ldfld char[] ConsoleApp1.Base::_res
    IL_000c: stloc.0
    IL_000d: ldc.i4.0
    IL_000e: ldelem.u2
    IL_000f: stloc.1
    IL_0010: ldloc.0
    IL_0011: ldc.i4.0
    IL_0012: ldarg.0
    IL_0013: ldloc.1
    IL_0014: callvirt instance char ConsoleApp1.Base::Do(char)
    IL_0019: stelem.i2
    IL_001a: ldloc.0
    IL_001b: ret
}
.method public hidebysig
    instance char[] DoWillDevirt () cil managed
{
    // Method begins at RVA 0x2098
    // Code size 25 (0x19)
    .maxstack 8

    IL_0000: ldarg.0
    IL_0001: ldfld char[] ConsoleApp1.Base::_src
    IL_0006: pop
    IL_0007: ldarg.0
    IL_0008: ldfld char[] ConsoleApp1.Base::_res
    IL_000d: dup
    IL_000e: ldc.i4.0
    IL_000f: ldarg.0
    IL_0010: ldc.i4.s 66
    IL_0012: callvirt instance char ConsoleApp1.Base::Do(char)
    IL_0017: stelem.i2
    IL_0018: ret
}

Build for coreclr was done at commit d4cab6e (from yesterday).

If this is already tracked in https://github.com/dotnet/coreclr/issues/9908 please feel free to close this issue here.

/cc: @AndyAyersMS

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

@AndyAyersMS
Copy link
Member

Taking a look.

@AndyAyersMS AndyAyersMS self-assigned this Jan 3, 2019
@AndyAyersMS
Copy link
Member

The slightly more complex IL in DoWontDevirt leads the inliner to conclude inlining it would not be profitable. And without inlining we won't devirtualize.

;; for DoWillDevirt

Inline candidate is mostly loads and stores.  Multiplier increased to 3.
Inline candidate callsite is boring.  Multiplier increased to 4.3.
calleeNativeSizeEstimate=240
callsiteNativeSizeEstimate=85
benefit multiplier=4.3
threshold=365
Native estimate for function size is within threshold for inlining 24 <= 36.5 (multiplier = 4.3)

;; for DoWontDevirt

Inline candidate is mostly loads and stores.  Multiplier increased to 3.
Inline candidate callsite is boring.  Multiplier increased to 4.3.
calleeNativeSizeEstimate=404
callsiteNativeSizeEstimate=85
benefit multiplier=4.3
threshold=365
Native estimate for function size exceeds threshold for inlining 40.4 > 36.5 (multiplier = 4.3)

One could imagine enhancing the inline heuristics so that if a final or exactly typed argument reaches a callvirt then there's an extra benefit added to encourage inlining. Added a note for this to #7541.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

4 participants