From: Andy Ayers Date: Mon, 29 Oct 2018 23:11:53 +0000 (-0700) Subject: JIT: streamline temp usage for returns (dotnet/coreclr#20640) X-Git-Tag: submit/tizen/20210909.063632~11030^2~3580 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=859e8b7e49eca88a01ca9d79dff890a13aa4cf6a;p=platform%2Fupstream%2Fdotnet%2Fruntime.git JIT: streamline temp usage for returns (dotnet/coreclr#20640) 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 dotnet/coreclr#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 dotnet/coreclr#15873. Commit migrated from https://github.com/dotnet/coreclr/commit/ccc18a6352c7a6232606131424c0377ea3529991 --- diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 7c5e264..b2846ed 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -5853,19 +5853,34 @@ void Compiler::fgFindBasicBlocks() // or if the inlinee has GC ref locals. if ((info.compRetNativeType != TYP_VOID) && ((retBlocks > 1) || impInlineInfo->HasGcRefLocals())) { - // The lifetime of this var might expand multiple BBs. So it is a long lifetime compiler temp. - lvaInlineeReturnSpillTemp = lvaGrabTemp(false DEBUGARG("Inline return value spill temp")); - lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType; + // If we've spilled the ret expr to a temp we can reuse the temp + // as the inlinee return spill temp. + // + // Todo: see if it is even better to always use this existing temp + // for return values, even if we otherwise wouldn't need a return spill temp... + lvaInlineeReturnSpillTemp = impInlineInfo->inlineCandidateInfo->preexistingSpillTemp; - // If the method returns a ref class, set the class of the spill temp - // to the method's return value. We may update this later if it turns - // out we can prove the method returns a more specific type. - if (info.compRetType == TYP_REF) + if (lvaInlineeReturnSpillTemp != BAD_VAR_NUM) + { + // This temp should already have the type of the return value. + JITDUMP("\nInliner: re-using pre-existing spill temp V%02u\n", lvaInlineeReturnSpillTemp); + } + else { - CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass; - if (retClassHnd != nullptr) + // The lifetime of this var might expand multiple BBs. So it is a long lifetime compiler temp. + lvaInlineeReturnSpillTemp = lvaGrabTemp(false DEBUGARG("Inline return value spill temp")); + lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType; + + // If the method returns a ref class, set the class of the spill temp + // to the method's return value. We may update this later if it turns + // out we can prove the method returns a more specific type. + if (info.compRetType == TYP_REF) { - lvaSetClass(lvaInlineeReturnSpillTemp, retClassHnd); + CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass; + if (retClassHnd != nullptr) + { + lvaSetClass(lvaInlineeReturnSpillTemp, retClassHnd); + } } } } diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index a8a1d01..1037c9d 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -2219,6 +2219,16 @@ bool Compiler::impSpillStackEntry(unsigned level, { CORINFO_CLASS_HANDLE stkHnd = verCurrentState.esStack[level].seTypeInfo.GetClassHandle(); lvaSetClass(tnum, tree, stkHnd); + + // If we're assigning a GT_RET_EXPR, note the temp over on the call, + // so the inliner can use it in case it needs a return spill temp. + if (tree->OperGet() == GT_RET_EXPR) + { + JITDUMP("\n*** see V%02u = GT_RET_EXPR, noting temp\n", tnum); + GenTree* call = tree->gtRetExpr.gtInlineCandidate; + InlineCandidateInfo* ici = call->gtCall.gtInlineCandidateInfo; + ici->preexistingSpillTemp = tnum; + } } // The tree type may be modified by impAssignTempGen, so use the type of the lclVar. @@ -18047,15 +18057,16 @@ void Compiler::impCheckCanInline(GenTree* call, InlineCandidateInfo* pInfo; pInfo = new (pParam->pThis, CMK_Inlining) InlineCandidateInfo; - pInfo->dwRestrictions = dwRestrictions; - pInfo->methInfo = methInfo; - pInfo->methAttr = pParam->methAttr; - pInfo->clsHandle = clsHandle; - pInfo->clsAttr = clsAttr; - pInfo->fncRetType = fncRetType; - pInfo->exactContextHnd = pParam->exactContextHnd; - pInfo->ilCallerHandle = pParam->pThis->info.compMethodHnd; - pInfo->initClassResult = initClassResult; + pInfo->dwRestrictions = dwRestrictions; + pInfo->methInfo = methInfo; + pInfo->methAttr = pParam->methAttr; + pInfo->clsHandle = clsHandle; + pInfo->clsAttr = clsAttr; + pInfo->fncRetType = fncRetType; + pInfo->exactContextHnd = pParam->exactContextHnd; + pInfo->ilCallerHandle = pParam->pThis->info.compMethodHnd; + pInfo->initClassResult = initClassResult; + pInfo->preexistingSpillTemp = BAD_VAR_NUM; *(pParam->ppInlineCandidateInfo) = pInfo; diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h index 0503b07..551341e 100644 --- a/src/coreclr/src/jit/inline.h +++ b/src/coreclr/src/jit/inline.h @@ -509,6 +509,7 @@ struct InlineCandidateInfo CORINFO_CONTEXT_HANDLE exactContextHnd; bool exactContextNeedsRuntimeLookup; CorInfoInitClassResult initClassResult; + unsigned preexistingSpillTemp; }; // InlArgInfo describes inline candidate argument properties. diff --git a/src/coreclr/tests/src/JIT/opt/Devirtualization/arraypool.cs b/src/coreclr/tests/src/JIT/opt/Devirtualization/arraypool.cs new file mode 100644 index 0000000..8d9b359 --- /dev/null +++ b/src/coreclr/tests/src/JIT/opt/Devirtualization/arraypool.cs @@ -0,0 +1,41 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Buffers; +using System.Runtime.CompilerServices; + +class X +{ + static int N; + static int J; + static int K; + + [MethodImpl(MethodImplOptions.NoInlining)] + static void S() { J = N / 2; K = J; } + + // We expect calls to Rent and Return to be + // devirtualized. + public static int Main() + { + N = 100; + byte[] buffer = ArrayPool.Shared.Rent(N); + int r = -1; + + try { + S(); + buffer[J] = 100; + r = (int) buffer[K]; + } + finally + { + if (buffer != null) + { + ArrayPool.Shared.Return(buffer); + } + } + + return r; + } +} diff --git a/src/coreclr/tests/src/JIT/opt/Devirtualization/arraypool.csproj b/src/coreclr/tests/src/JIT/opt/Devirtualization/arraypool.csproj new file mode 100644 index 0000000..d5105da --- /dev/null +++ b/src/coreclr/tests/src/JIT/opt/Devirtualization/arraypool.csproj @@ -0,0 +1,38 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + False + + + + None + True + + + + + + + + + + \ No newline at end of file diff --git a/src/coreclr/tests/src/JIT/opt/Devirtualization/late1.cs b/src/coreclr/tests/src/JIT/opt/Devirtualization/late1.cs new file mode 100644 index 0000000..69508d1 --- /dev/null +++ b/src/coreclr/tests/src/JIT/opt/Devirtualization/late1.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +public class B +{ + public virtual int F(int x) { return x + 33; } +} + +public class D : B +{ + public override int F(int x) { return x + 44; } +} + +public class Q +{ + static int V(int x) + { + return x; + } + + // calls to B will use a return spill temp since + // B has multiple return sites. + // + // Jit will initially type this temp as 'B' but then + // sharpen type type to 'B exact' or 'D exact' if the + // 'b' is a constant at the call site. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + static B Choose(bool b) + { + return b ? new B() : new D(); + } + + // The calls to F should be devirtualized late + public static int Main(string[] args) + { + int v0 = Choose(false).F(V(67)); + B b = Choose(true); + int v1 = b.F(V(56)); + return v0 + v1 - 100; + } +} diff --git a/src/coreclr/tests/src/JIT/opt/Devirtualization/late1.csproj b/src/coreclr/tests/src/JIT/opt/Devirtualization/late1.csproj new file mode 100644 index 0000000..66a4233 --- /dev/null +++ b/src/coreclr/tests/src/JIT/opt/Devirtualization/late1.csproj @@ -0,0 +1,38 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + False + + + + None + True + + + + + + + + + + \ No newline at end of file