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

JIT: Improve inliner (constant feeds constant test, foldable calls) #50784

Closed
wants to merge 4 commits into from

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Apr 6, 2021

Contribution to @tannergooding's work in #50675

static void Test()
{
    CheckHW();
}

static void CheckHW()
{
    if (!Avx2.IsSupported)
    {
        Console.WriteLine("No AVX2 :(");
    }
}

Current codegen for Test:

G_M24707_IG01:           
       4883EC28             sub      rsp, 40
G_M24707_IG02:             
       E8EF5CFEFF           call     CheckHW()  ;; <-- not inlined
       90                   nop
G_M24707_IG03:         
       4883C428             add      rsp, 40
       C3                   ret


; weight= 79 : state  40 [ call ]
; weight= 25 : state  45 [ brtrue.s ]
; weight= 66 : state 102 [ ldstr ]
; weight= 79 : state  40 [ call ]
; weight= 19 : state  42 [ ret ]
; 
; Inline candidate looks like a wrapper method.  Multiplier increased to 1.
; Inline candidate callsite is boring.  Multiplier increased to 2.3.
; calleeNativeSizeEstimate=268
; callsiteNativeSizeEstimate=55
; benefit multiplier=2.3
; threshold=126
; Native estimate for function size exceeds threshold for inlining 26.8 > 12.6 (multiplier = 2.3)

New codegen for Test:

G_M24707_IG01:           
G_M24707_IG02:             
       C3                   ret


; weight= 79 : state  40 [ call ]
; weight= 25 : state  45 [ brtrue.s ]
; weight= 66 : state 102 [ ldstr ]
; weight= 79 : state  40 [ call ]
; weight= 19 : state  42 [ ret ]
; 
; Inline candidate looks like a wrapper method.  Multiplier increased to 1.
; Inline candidate has an arg that feeds a constant test.  Multiplier increased to 2.
; Inline candidate has 1 call(s) which is guaranteed to be folded
; Inline candidate has const arg that feeds a conditional.  Multiplier increased to 5.
; Inline candidate callsite is boring.  Multiplier increased to 6.3.
; calleeNativeSizeEstimate=268
; callsiteNativeSizeEstimate=55
; benefit multiplier=6.3
; threshold=346
; Native estimate for function size is within threshold for inlining 26.8 <= 34.6 (multiplier = 6.3)

As far as I understand we're going to switch to "PGO-based policy" for inliner in future but maybe some of these pieces will be useful there too?

@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 Apr 6, 2021
@@ -66,6 +66,7 @@ INLINE_OBSERVATION(TOO_MUCH_IL, bool, "too many il bytes",

// ------ Callee Information -------

INLINE_OBSERVATION(FOLDABLE_CALL, bool, "foldable call", INFORMATION, CALLEE)
Copy link
Member

Choose a reason for hiding this comment

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

For some future PR, we could also track the intrinsic id here.

This would allow us to determine, at least the following interesting pieces:

  • Number of intrinsic calls
  • Number of hardware intrinsic calls
  • Number of Type.op_Equality calls

There may be other interesting bits as well, but these are at least ones that have one or more of:

  • significantly decreased cost (hwintrinsics are instructions not calls)
  • additional optimizations (many intrinsics have CSE support)
  • potential dead code elimination (type equality checks, particularly where T : struct)

@EgorBo EgorBo marked this pull request as ready for review April 7, 2021 20:33
@AndyAyersMS
Copy link
Member

This is now going to require making String.Length a new-style intrinsic.

@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.0 milestone Apr 15, 2021
@EgorBo
Copy link
Member Author

EgorBo commented May 4, 2021

Will re-think this one in a new PR.

@EgorBo EgorBo closed this May 4, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 3, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants