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

Conversation

@benaadams
Copy link
Member

The conversion doesn't currently inline producing

       lea      rcx, bword ptr [rsp+58H]
       mov      rdx, rdi
       call     String:op_Implicit(ref):struct
       lea      rcx, bword ptr [rsp+48H]
       mov      rdx, rbx
       call     String:op_Implicit(ref):struct
       lea      rcx, bword ptr [rsp+38H]
       mov      rdx, bword ptr [rsp+58H]
       mov      bword ptr [rcx], rdx
       mov      edx, dword ptr [rsp+60H]
       mov      dword ptr [rcx+8], edx
       lea      rcx, bword ptr [rsp+28H]
       mov      rdx, bword ptr [rsp+48H]
       mov      bword ptr [rcx], rdx
       mov      edx, dword ptr [rsp+50H]
       mov      dword ptr [rcx+8], edx
       lea      rcx, bword ptr [rsp+38H]
       lea      rdx, bword ptr [rsp+28H]
       call     CompareInfo:CompareOrdinalIgnoreCase(struct,struct):int

When inlined this reduces to

       lea      rax, bword ptr [rdi+12]
       lea      r8, bword ptr [rbx+12]
       lea      r9, bword ptr [rsp+38H]
       mov      bword ptr [r9], rax
       mov      dword ptr [r9+8], ecx
       lea      rcx, bword ptr [rsp+28H]
       mov      bword ptr [rcx], r8
       mov      dword ptr [rcx+8], edx
       lea      rcx, bword ptr [rsp+38H]
       lea      rdx, bword ptr [rsp+28H]
       call     CompareInfo:CompareOrdinalIgnoreCase(struct,struct):int

/cc @jkotas @stephentoub

@stephentoub
Copy link
Member

cc: @AndyAyersMS

@benaadams
Copy link
Member Author

benaadams commented Feb 18, 2018

Currently its gets pre-marked as no-inline

Marking String:op_Implicit(ref):struct as NOINLINE because of unprofitable inline
Successfully inlined String:GetRawStringData():byref:this (7 IL bytes) (depth 1) [below ALWAYS_INLINE size]
Successfully inlined ReadOnlySpan`1:.ctor(byref,int):this (20 IL bytes) (depth 1) [aggressive inline attribute]
**************** Inline Tree
Inlines into 06000217 String:op_Implicit(ref):struct
  [1 IL=0014 TR=000008 06000236] [below ALWAYS_INLINE size] String:GetRawStringData():byref:this
  [2 IL=0025 TR=000019 060014E4] [aggressive inline attribute] ReadOnlySpan`1:.ctor(byref,int):this

Then all following occurances

[FAILED: noinline per IL/cached result] String:op_Implicit(ref):struct

@stephentoub stephentoub merged commit 9b0a9fd into dotnet:master Feb 18, 2018
@AndyAyersMS
Copy link
Member

Looks like the prejit inline evaluation just missed thinking this was a good candidate:

Inline candidate has an arg that feeds a constant test.  Multiplier increased to 1.
Prejit root candidate has arg that feeds a conditional.  Multiplier increased to 4.
Inline candidate callsite is hot.  Multiplier increased to 7.
calleeNativeSizeEstimate=614
callsiteNativeSizeEstimate=85
benefit multiplier=7
threshold=595
Native estimate for function size exceeds threshold for inlining 61.4 > 59.5 (multiplier = 7)
INLINER: during 'prejit' result 'failed this callee' reason 'unprofitable inline' for 'n/a' calling 'String:op_Implicit(ref):struct'

If this method was a constructor instead of a conversion operator it would have gotten an additional benefit boost and would not have been preemptively classified as no inline. So for consistency's sake, it might be worth looking at costing implicit conversions as if they were constructors. Or perhaps or in addition, giving an extra benefit boost to methods taking or returning span-like values.

@benaadams benaadams deleted the inline-string-span branch February 19, 2018 01:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants