Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

JIT: refine types when creating arg temps to improve devirtualization #13530

Merged
merged 3 commits into from
Aug 23, 2017

Conversation

AndyAyersMS
Copy link
Member

The jit will refine the types of temps used to pass arguments to inlinees
when it creates the assignments to these temps.

Unfortunately this is too late to drive devirtualization in the body of
the inlinee, as thes assignments are created after the inlinee body is
imported.

So, add similar refinement logic to the place where the temps are first
created.

Closes #13520.

The jit will refine the types of temps used to pass arguments to inlinees
when it creates the assignments to these temps.

Unfortunately this is too late to drive devirtualization in the body of
the inlinee, as thes assignments are created after the inlinee body is
imported.

So, add similar refinement logic to the place where the temps are first
created.

Closes #13520.
@AndyAyersMS
Copy link
Member Author

@JosephTremoulet PTAL
cc @dotnet/jit-contrib

Leads to around 50 more devirtualizations in corelib + fx per jit diffs:

Total bytes of diff: -264 (0.00 % of base)
    diff is an improvement.
Total byte diff includes 0 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    0 unique methods,        0 unique bytes
Top file improvements by size (bytes):
        -167 : System.Private.CoreLib.dasm (0.00 % of base)
         -67 : Microsoft.CodeAnalysis.dasm (-0.01 % of base)
         -18 : Microsoft.CodeAnalysis.VisualBasic.dasm (0.00 % of base)
         -12 : Microsoft.CodeAnalysis.CSharp.dasm (0.00 % of base)
4 total files with size differences (4 improved, 0 regressed), 75 unchanged.
Top method regessions by size (bytes):
           6 : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.Binder:BindInvocationExpression(ref,ref):ref:this
           6 : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.Binder:BindClassCreationExpression(ref,ref,ref,ref,ref):ref:this
           6 : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.Binder:BindDelegateCreationExpression(ref,ref,ref):ref:this
Top method improvements by size (bytes):
         -19 : System.Private.CoreLib.dasm - System.IO.FileStream:Init(int,int):this
         -12 : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.PreciseAbstractFlowPass`1[LocalState][Microsoft.CodeAnalysis.CSharp.ControlFlowPass+LocalState]:VisitSwitchHeader(ref):struct:this
         -12 : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.PreciseAbstractFlowPass`1[LocalState][Microsoft.CodeAnalysis.CSharp.DataFlowPass+LocalState]:VisitSwitchHeader(ref):struct:this
         -12 : Microsoft.CodeAnalysis.VisualBasic.dasm - Microsoft.CodeAnalysis.VisualBasic.Conversions:ClassifyQueryLambdaConversion(ref,ref,ref,byref):int
          -9 : System.Private.CoreLib.dasm - System.Reflection.Emit.EnumBuilder:GetAttributeFlagsImpl():int:this
51 total methods with size differences (48 improved, 3 regressed), 78387 unchanged.

Also fixes the case in #13520; inline + devirt trace now shows:

Inlines into 06000002 J:Main():int
  [1 IL=0000 TR=000007 06000003] [profitable inline] J:.ctor():this
    [2 IL=0002 TR=000023 06000007] [below ALWAYS_INLINE size] Increment:.ctor(int):this
      [3 IL=0001 TR=000035 06000006] [below ALWAYS_INLINE size] Operation:.ctor():this
        [4 IL=0001 TR=000044 0600011A] [below ALWAYS_INLINE size] System.Object:.ctor():this
    [5 IL=0013 TR=000031 0600011A] [below ALWAYS_INLINE size] System.Object:.ctor():this
  [6 IL=0005 TR=000010 06000001] [below ALWAYS_INLINE size] J:CallVirtualMethod():int:this
    [7 IL=0008 TR=000054 06000005] [below ALWAYS_INLINE size] Operation:OperateTwice(int):int:this
      [8 IL=0003 TR=000063 06000008] [below ALWAYS_INLINE size] Increment:Operate(int):int:this
      [9 IL=0008 TR=000067 06000008] [below ALWAYS_INLINE size] Increment:Operate(int):int:this

Copy link

@JosephTremoulet JosephTremoulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Does this mean we don't need the code to improve the type when generating the assign anymore, or does that help in more situations than this does?

@AndyAyersMS
Copy link
Member Author

With current jit logic, it should no longer be needed.

Think I should remove it or turn it into some kind of insertion?

@JosephTremoulet
Copy link

I'd be inclined to just remove it; my hunch about making it an assertion would be that it would never trip or else it would trip in some esoteric cases that aren't really that interesting and could end up needing excessive bookkeeping to keep the assertion happy. But, that's just my guess, I'd be fine with whichever way you want to go.

@AndyAyersMS
Copy link
Member Author

Yeah, an assertion could be a bit tricky, because we currently know less when creating the assignment than we do when we create the temp (no stack entry to consult).

Let me remove the downstream update and verify there are no diffs.

@AndyAyersMS
Copy link
Member Author

No diffs, so have removed the update-during-assign code.

@AndyAyersMS AndyAyersMS merged commit 85c59f9 into dotnet:master Aug 23, 2017
@karelz karelz modified the milestone: 2.1.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants