From 0bb26f32135e633fd1533b941f3e11a0f27a06e7 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 16 Mar 2018 17:49:39 -0700 Subject: [PATCH 1/2] JIT: remove boxing for interface call to shared generic struct Improve the jit's ability to optimize a box-interface call sequence to handle cases where the unboxed entry point requires a method table argument. Added two test cases, one from the inspiring issue, and another where the jit is then able to inline the method. Closes #16982. --- src/jit/compiler.h | 3 +- src/jit/gentree.cpp | 7 +++- src/jit/importer.cpp | 42 ++++++++++++++++--- tests/src/JIT/opt/Devirtualization/box1.cs | 29 +++++++++++++ .../src/JIT/opt/Devirtualization/box1.csproj | 34 +++++++++++++++ tests/src/JIT/opt/Devirtualization/box2.cs | 24 +++++++++++ .../src/JIT/opt/Devirtualization/box2.csproj | 35 ++++++++++++++++ 7 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 tests/src/JIT/opt/Devirtualization/box1.cs create mode 100644 tests/src/JIT/opt/Devirtualization/box1.csproj create mode 100644 tests/src/JIT/opt/Devirtualization/box2.cs create mode 100644 tests/src/JIT/opt/Devirtualization/box2.csproj diff --git a/src/jit/compiler.h b/src/jit/compiler.h index f1c952920574..7138107dd90d 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -2270,7 +2270,8 @@ class Compiler BR_REMOVE_AND_NARROW, // remove effects, minimize remaining work, return possibly narrowed source tree BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE, // remove effects and minimize remaining work, return type handle tree BR_REMOVE_BUT_NOT_NARROW, // remove effects, return original source tree - BR_DONT_REMOVE, // just check if removal is possible + BR_DONT_REMOVE, // check if removal is possible, return copy source tree + BR_DONT_REMOVE_WANT_TYPE_HANDLE, // check if removal is possible, return type handle tree BR_MAKE_LOCAL_COPY // revise box to copy to temp local and return local's address }; diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 41733d0de857..41de817ad106 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -13468,7 +13468,7 @@ GenTree* Compiler::gtTryRemoveBoxUpstreamEffects(GenTree* op, BoxRemovalOptions // If we're eventually going to return the type handle, remember it now. GenTree* boxTypeHandle = nullptr; - if (options == BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE) + if ((options == BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE) || (options == BR_DONT_REMOVE_WANT_TYPE_HANDLE)) { GenTree* asgSrc = asg->gtOp.gtOp2; genTreeOps asgSrcOper = asgSrc->OperGet(); @@ -13632,6 +13632,11 @@ GenTree* Compiler::gtTryRemoveBoxUpstreamEffects(GenTree* op, BoxRemovalOptions return copySrc; } + if (options == BR_DONT_REMOVE_WANT_TYPE_HANDLE) + { + return boxTypeHandle; + } + // Otherwise, proceed with the optimization. // // Change the assignment expression to a NOP. diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 30faec23b874..981a0d7a21dd 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -19502,8 +19502,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, } // Fetch method attributes to see if method is marked final. - const DWORD derivedMethodAttribs = info.compCompHnd->getMethodAttribs(derivedMethod); - const bool derivedMethodIsFinal = ((derivedMethodAttribs & CORINFO_FLG_FINAL) != 0); + DWORD derivedMethodAttribs = info.compCompHnd->getMethodAttribs(derivedMethod); + const bool derivedMethodIsFinal = ((derivedMethodAttribs & CORINFO_FLG_FINAL) != 0); #if defined(DEBUG) const char* derivedClassName = "?derivedClass"; @@ -19597,7 +19597,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, JITDUMP("Now have direct call to boxed entry point, looking for unboxed entry point\n"); // Note for some shared methods the unboxed entry point requires an extra parameter. - // We defer optimizing if so. bool requiresInstMethodTableArg = false; CORINFO_METHOD_HANDLE unboxedEntryMethod = info.compCompHnd->getUnboxedEntry(derivedMethod, &requiresInstMethodTableArg); @@ -19614,9 +19613,40 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // the copy, we can undo the copy too. if (requiresInstMethodTableArg) { - // We can likely handle this case by grabbing the argument passed to - // the newobj in the box. But defer for now. - JITDUMP("Found unboxed entry point, but it needs method table arg, deferring\n"); + // Perform a trial box removal and ask for the type handle tree. + JITDUMP("Unboxed entry needs method table arg...\n"); + GenTree* methodTableArg = gtTryRemoveBoxUpstreamEffects(thisObj, BR_DONT_REMOVE_WANT_TYPE_HANDLE); + + if (methodTableArg != nullptr) + { + // If that worked, turn the box into a copy to a local var + JITDUMP("Found suitable method table arg tree [%06u]\n", dspTreeID(methodTableArg)); + GenTree* localCopyThis = gtTryRemoveBoxUpstreamEffects(thisObj, BR_MAKE_LOCAL_COPY); + + if (localCopyThis != nullptr) + { + // Pass the local var as this and the type handle as a new arg + JITDUMP("Success! invoking unboxed entry point on local copy, and passing method table arg\n"); + call->gtCallObjp = localCopyThis; + call->gtCallArgs = gtNewListNode(methodTableArg, call->gtCallArgs); + call->gtCallMethHnd = unboxedEntryMethod; + derivedMethod = unboxedEntryMethod; + + // Method attributes will differ because unboxed entry point is shared + const DWORD unboxedMethodAttribs = info.compCompHnd->getMethodAttribs(unboxedEntryMethod); + JITDUMP("Updating method attribs from 0x%08x to 0x%08x\n", derivedMethodAttribs, + unboxedMethodAttribs); + derivedMethodAttribs = unboxedMethodAttribs; + } + else + { + JITDUMP("Sorry, failed to undo the box -- can't convert to local copy\n"); + } + } + else + { + JITDUMP("Sorry, failed to undo the box -- can't find method table arg\n"); + } } else { diff --git a/tests/src/JIT/opt/Devirtualization/box1.cs b/tests/src/JIT/opt/Devirtualization/box1.cs new file mode 100644 index 000000000000..51121f37d8b3 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/box1.cs @@ -0,0 +1,29 @@ +// 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; + +interface IPrint +{ + void Print(); +} + +struct X : IPrint +{ + public X(T t) { _t = t; } + public void Print() { Console.WriteLine(_t); } + T _t; +} + +class Y +{ + static int Main() + { + var s = new X("hello, world!"); + // Jit should devirtualize, remove box, + // change to call unboxed entry, then inline. + ((IPrint)s).Print(); + return 100; + } +} diff --git a/tests/src/JIT/opt/Devirtualization/box1.csproj b/tests/src/JIT/opt/Devirtualization/box1.csproj new file mode 100644 index 000000000000..a1390b0c17ab --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/box1.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + None + True + + + + + + + + + + \ No newline at end of file diff --git a/tests/src/JIT/opt/Devirtualization/box2.cs b/tests/src/JIT/opt/Devirtualization/box2.cs new file mode 100644 index 000000000000..9e82ef2ff52b --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/box2.cs @@ -0,0 +1,24 @@ +// 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.Threading; +using System.Threading.Tasks; + +class Program +{ + static async Task Main() + { + for (int i = 0; i < 10; i++) + { + // In the associated AwaitUnsafeOnCompleted, the + // jit should devirtualize, remove the box, + // and change to call unboxed entry, passing + // extra context argument. + await new ValueTask(Task.Delay(1).ContinueWith(_ => default(string))).ConfigureAwait(false); + } + + return 100; + } +} diff --git a/tests/src/JIT/opt/Devirtualization/box2.csproj b/tests/src/JIT/opt/Devirtualization/box2.csproj new file mode 100644 index 000000000000..e821b3bca9d2 --- /dev/null +++ b/tests/src/JIT/opt/Devirtualization/box2.csproj @@ -0,0 +1,35 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + 7.2 + None + True + + + + + + + + + + \ No newline at end of file From 91c40782ca7e20f5a216b4b0129fd1562433d45b Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 16 Mar 2018 21:20:11 -0700 Subject: [PATCH 2/2] Handle L2R arg passing --- src/jit/importer.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 981a0d7a21dd..c23f17c350c6 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -19627,8 +19627,25 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, { // Pass the local var as this and the type handle as a new arg JITDUMP("Success! invoking unboxed entry point on local copy, and passing method table arg\n"); - call->gtCallObjp = localCopyThis; - call->gtCallArgs = gtNewListNode(methodTableArg, call->gtCallArgs); + call->gtCallObjp = localCopyThis; + + // Prepend for R2L arg passing or empty L2R passing + if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr)) + { + call->gtCallArgs = gtNewListNode(methodTableArg, call->gtCallArgs); + } + // Append for non-empty L2R + else + { + GenTreeArgList* beforeArg = call->gtCallArgs; + while (beforeArg->Rest() != nullptr) + { + beforeArg = beforeArg->Rest(); + } + + beforeArg->Rest() = gtNewListNode(methodTableArg, nullptr); + } + call->gtCallMethHnd = unboxedEntryMethod; derivedMethod = unboxedEntryMethod;