JIT: streamline temp usage for returns

[AndyAyersMS]

If the jit decides it needs a return spill temp, and the return value
has already been spilled to a single-def temp, re-use the existing
for the return temp rather than creating a new one.

In conjunction with #20553 this allows late devirtualization for calls where
the object in the virtual call is the result of an inline that provides
a better type, and the objected formerly reached the call via one or more
intermediate temps.

Closes #15783.

[AndyAyersMS]

This was originally needed for devirtualizing ArrayPool<T>.Shared.Rent but other jit changes have taken care of that. But it still has some benefit on its own.

Diffs generally favorable; often better spill placement (one spill below a join instead of two spills above).

The one big regression is in a method where inlining halts because of too many local vars. Since we've reduced temp usage somewhat we now do more inlines before we hit the limit.

Total bytes of diff: -227 (0.00% of base)
    diff is an improvement.

Top file regressions by size (bytes):
          47 : Microsoft.CodeAnalysis.VisualBasic.dasm (0.00% of base)
          42 : Microsoft.DotNet.ProjectModel.dasm (0.02% of base)
          31 : System.Private.Xml.dasm (0.00% of base)
          19 : Microsoft.CodeAnalysis.dasm (0.00% of base)
           6 : System.Data.Common.dasm (0.00% of base)

Top file improvements by size (bytes):
        -237 : Microsoft.CodeAnalysis.CSharp.dasm (-0.01% of base)
        -105 : System.Private.DataContractSerialization.dasm (-0.01% of base)
         -13 : System.Collections.dasm (0.00% of base)
         -11 : Microsoft.CSharp.dasm (0.00% of base)
         -11 : NuGet.Packaging.dasm (-0.01% of base)

13 total files with size differences (6 improved, 7 regressed), 116 unchanged.

Top method regressions by size (bytes):
         569 ( 3.34% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:ReportOverloadResolutionFailureForASingleCandidate(ref,ref,int,byref,struct,struct,bool,bool,bool,bool,ref,ref,bool,ref,ref):this
          36 ( 0.62% of base) : Microsoft.DotNet.ProjectModel.dasm - LibraryManager:GetAllDiagnostics():ref:this
          25 ( 0.98% of base) : System.Private.Xml.dasm - SchemaCollectionCompiler:CompileAttribute(ref):this
          15 ( 0.09% of base) : System.Private.Xml.dasm - XmlReflectionImporter:ImportAccessorMapping(ref,ref,ref,ref,ref,bool,bool,ref):this
           6 ( 0.26% of base) : System.Data.Common.dasm - XSDSchema:HandleRelation(ref,bool):this

Top method improvements by size (bytes):
         -91 (-0.87% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceMemberContainerTypeSymbol:AddNonTypeMembers(ref,struct,ref):this
         -60 (-0.53% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - SourceMemberFieldSymbol:Create(ref,ref,ref,ref,byref,byref,ref)
         -49 (-0.53% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:DecodeParameterList(ref,bool,int,struct,ref,ref,ref):this
         -30 (-0.79% of base) : System.Private.DataContractSerialization.dasm - XmlBinaryReader:ReadArray(ref,ref,ref,int,int):int:this (20 methods)
         -30 (-1.23% of base) : System.Private.DataContractSerialization.dasm - XmlDictionaryReader:ReadArray(ref,ref,ref,int,int):int:this (20 methods)

Top method regressions by size (percentage):
         569 ( 3.34% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:ReportOverloadResolutionFailureForASingleCandidate(ref,ref,int,byref,struct,struct,bool,bool,bool,bool,ref,ref,bool,ref,ref):this
           3 ( 1.61% of base) : Microsoft.DotNet.ProjectModel.dasm - PatternContextLinear:CalculateStem(ref):ref:this
           3 ( 1.58% of base) : Microsoft.DotNet.ProjectModel.dasm - PatternContextRagged:CalculateStem(ref):ref:this
           4 ( 1.40% of base) : Microsoft.CodeAnalysis.dasm - LocalSlotManager:AllocateSlot(ref,ubyte,struct):ref:this
           2 ( 1.32% of base) : Microsoft.CodeAnalysis.dasm - LocalSlotManager:FreeSlot(ref):this

Top method improvements by size (percentage):
          -3 (-6.12% of base) : System.Private.DataContractSerialization.dasm - XmlDictionaryReader:GetAttribute(ref,ref):ref:this
          -3 (-5.77% of base) : System.Private.DataContractSerialization.dasm - XmlDictionaryReader:IsStartElement(ref,ref):bool:this
          -9 (-4.29% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Scanner:XmlMakeEndEmbeddedToken(struct,ref):ref:this
          -2 (-4.08% of base) : System.Private.DataContractSerialization.dasm - XmlDictionaryWriter:WriteStartElement(ref,ref,ref):this
          -2 (-4.08% of base) : System.Private.DataContractSerialization.dasm - XmlDictionaryWriter:WriteStartAttribute(ref,ref,ref):this

160 total methods with size differences (138 improved, 22 regressed), 192533 unchanged.

cc @dotnet/jit-contrib

[AndyAyersMS]

Diff from the "late1" test case

        call     CORINFO_HELP_NEWSFAST
        mov      rcx, rax
        mov      edx, 67
-       mov      rax, qword ptr [rax]
-       mov      rax, qword ptr [rax+64]
-       call     qword ptr [rax+32]B:F(int):int:this
+       call     D:F(int):int:this
        mov      esi, eax
        mov      rcx, 0xD1FFAB1E
        call     CORINFO_HELP_NEWSFAST
        mov      rcx, rax
        mov      edx, 56
-       mov      rax, qword ptr [rax]
-       mov      rax, qword ptr [rax+64]
-       call     qword ptr [rax+32]B:F(int):int:this
+       call     B:F(int):int:this
        lea      eax, [rsi+rax-100]

[AndyAyersMS]

Typical OSX CI failure:

14:49:29  > git fetch --tags --progress https://github.com/dotnet/coreclr +refs/heads/*:refs/remotes/origin/* # timeout=90
15:37:31 FATAL: java.nio.channels.ClosedChannelException
15:37:31 java.nio.channels.ClosedChannelException
15:37:31 Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to JNLP4-connect connection from 131.107.160.23/131.107.160.23:32054
15:37:31 		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1693)
15:37:31 		at hudson.remoting.Request.call(Request.java:192)

[benaadams]

Not sure the close of #15873 "[Local GC] FEATURE_EVENT_TRACE 1/n: Tracking Event State" is the right issue?

[benaadams]

this allows late devirtualization for calls where the object in the virtual call is the result of an inline that provides a better type, and the objected formerly reached the call via one or more intermediate temps.

Would it allow a second devirtualization (e.g. in dotnet/corefx#33085)

[AndyAyersMS]

@benaadams thanks -- that should have been #15783. Edited the PR header and will try and remember to edit the commit message too.

[AndyAyersMS]

Would it allow a second devirtualization (e.g. in dotnet/corefx#33085)

If I understand the example correctly we would need late devirtualization to inspire more inlining. We're not able to do that today.

[benaadams]

If I understand the example correctly we would need late devirtualization to inspire more inlining. We're not able to do that today.

Might not be very common (that's the only example that comes to mind atm)

Further down the devirtualization line via interfaces might make it more of a thing; like passing a List<T> into a Linq IEnumerable<T> and second staging through IEnumerator<T>. Though the methods would probably be too large to opportunistically inline; as well as containing loops, so again might not be common.

[AndyAyersMS]

@dotnet/jit-contrib PTAL

[briansull]

Looks Good