From f249a3d58b0b2bb55adca1a56c5a39cf446437b7 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 2 Apr 2022 00:25:56 +0300 Subject: [PATCH] Rely on PGO for isinst/castclass (#65922) --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/importer.cpp | 92 ++++++-- src/coreclr/jit/jitconfigvalues.h | 6 +- .../ProfileCastClassAndIsInst.cs | 214 ++++++++++++++++++ .../ProfileCastClassAndIsInst.csproj | 23 ++ .../ProfileCastClassAndIsInst_random1.csproj | 25 ++ .../ProfileCastClassAndIsInst_random2.csproj | 25 ++ .../ProfileCastClassAndIsInst_random3.csproj | 25 ++ 8 files changed, 396 insertions(+), 16 deletions(-) create mode 100644 src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.cs create mode 100644 src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.csproj create mode 100644 src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random1.csproj create mode 100644 src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random2.csproj create mode 100644 src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random3.csproj diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 4daea0334752c..665d1884978bf 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4639,7 +4639,7 @@ class Compiler CORINFO_LOOKUP_KIND* pGenericLookupKind = nullptr); bool impIsCastHelperEligibleForClassProbe(GenTree* tree); - bool impIsCastHelperMayHaveProfileData(GenTree* tree); + bool impIsCastHelperMayHaveProfileData(CorInfoHelpFunc helper); GenTree* impCastClassOrIsInstToTree( GenTree* op1, GenTree* op2, CORINFO_RESOLVED_TOKEN* pResolvedToken, bool isCastClass, IL_OFFSET ilOffset); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c0ab1d4bfc82e..b462f6807c6e5 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2111,7 +2111,7 @@ GenTree* Compiler::impReadyToRunLookupToTree(CORINFO_CONST_LOOKUP* pLookup, // bool Compiler::impIsCastHelperEligibleForClassProbe(GenTree* tree) { - if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) || (JitConfig.JitCastProfiling() != 1)) + if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) || (JitConfig.JitProfileCasts() != 1)) { return false; } @@ -2138,21 +2138,22 @@ bool Compiler::impIsCastHelperEligibleForClassProbe(GenTree* tree) // Returns: // true if the tree is a cast helper with potential profile data // -bool Compiler::impIsCastHelperMayHaveProfileData(GenTree* tree) +bool Compiler::impIsCastHelperMayHaveProfileData(CorInfoHelpFunc helper) { - if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT) || (JitConfig.JitCastProfiling() != 1)) + if (JitConfig.JitConsumeProfileForCasts() == 0) { return false; } - if (tree->IsCall() && tree->AsCall()->gtCallType == CT_HELPER) + if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT)) { - const CorInfoHelpFunc helper = eeGetHelperNum(tree->AsCall()->gtCallMethHnd); - if ((helper == CORINFO_HELP_ISINSTANCEOFINTERFACE) || (helper == CORINFO_HELP_ISINSTANCEOFCLASS) || - (helper == CORINFO_HELP_CHKCASTCLASS) || (helper == CORINFO_HELP_CHKCASTINTERFACE)) - { - return true; - } + return false; + } + + if ((helper == CORINFO_HELP_ISINSTANCEOFINTERFACE) || (helper == CORINFO_HELP_ISINSTANCEOFCLASS) || + (helper == CORINFO_HELP_CHKCASTCLASS) || (helper == CORINFO_HELP_CHKCASTINTERFACE)) + { + return true; } return false; } @@ -11603,7 +11604,7 @@ GenTree* Compiler::impCastClassOrIsInstToTree( // not worth creating an untracked local variable shouldExpandInline = false; } - else if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) && (JitConfig.JitCastProfiling() == 1)) + else if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) && (JitConfig.JitProfileCasts() == 1)) { // Optimizations are enabled but we're still instrumenting (including casts) if (isCastClass && !impIsClassExact(pResolvedToken->hClass)) @@ -11616,8 +11617,11 @@ GenTree* Compiler::impCastClassOrIsInstToTree( // Pessimistically assume the jit cannot expand this as an inline test bool canExpandInline = false; + bool partialExpand = false; const CorInfoHelpFunc helper = info.compCompHnd->getCastingHelper(pResolvedToken, isCastClass); + GenTree* exactCls = nullptr; + // Legality check. // // Not all classclass/isinst operations can be inline expanded. @@ -11637,6 +11641,61 @@ GenTree* Compiler::impCastClassOrIsInstToTree( canExpandInline = impIsClassExact(pResolvedToken->hClass); } } + + // Check if this cast helper have some profile data + if (impIsCastHelperMayHaveProfileData(helper)) + { + bool doRandomDevirt = false; + const int maxLikelyClasses = 32; + int likelyClassCount = 0; + LikelyClassRecord likelyClasses[maxLikelyClasses]; +#ifdef DEBUG + // Optional stress mode to pick a random known class, rather than + // the most likely known class. + doRandomDevirt = JitConfig.JitRandomGuardedDevirtualization() != 0; + + if (doRandomDevirt) + { + // Reuse the random inliner's random state. + CLRRandom* const random = + impInlineRoot()->m_inlineStrategy->GetRandom(JitConfig.JitRandomGuardedDevirtualization()); + likelyClasses[0].clsHandle = getRandomClass(fgPgoSchema, fgPgoSchemaCount, fgPgoData, ilOffset, random); + likelyClasses[0].likelihood = 100; + if (likelyClasses[0].clsHandle != NO_CLASS_HANDLE) + { + likelyClassCount = 1; + } + } + else +#endif + { + likelyClassCount = getLikelyClasses(likelyClasses, maxLikelyClasses, fgPgoSchema, fgPgoSchemaCount, + fgPgoData, ilOffset); + } + + if (likelyClassCount > 0) + { + LikelyClassRecord likelyClass = likelyClasses[0]; + CORINFO_CLASS_HANDLE likelyCls = likelyClass.clsHandle; + + if ((likelyCls != NO_CLASS_HANDLE) && + (likelyClass.likelihood > (UINT32)JitConfig.JitGuardedDevirtualizationChainLikelihood())) + { + if ((info.compCompHnd->compareTypesForCast(likelyCls, pResolvedToken->hClass) == + TypeCompareState::Must)) + { + assert((info.compCompHnd->getClassAttribs(likelyCls) & + (CORINFO_FLG_INTERFACE | CORINFO_FLG_ABSTRACT)) == 0); + JITDUMP("Adding \"is %s (%X)\" check as a fast path for %s using PGO data.\n", + eeGetClassName(likelyCls), likelyCls, isCastClass ? "castclass" : "isinst"); + + canExpandInline = true; + partialExpand = true; + exactCls = gtNewIconEmbClsHndNode(likelyCls); + } + } + } + } } const bool expandInline = canExpandInline && shouldExpandInline; @@ -11688,13 +11747,13 @@ GenTree* Compiler::impCastClassOrIsInstToTree( // GenTree* op2Var = op2; - if (isCastClass) + if (isCastClass && !partialExpand) { op2Var = fgInsertCommaFormTemp(&op2); lvaTable[op2Var->AsLclVarCommon()->GetLclNum()].lvIsCSE = true; } temp = gtNewMethodTableLookup(temp); - condMT = gtNewOperNode(GT_NE, TYP_INT, temp, op2); + condMT = gtNewOperNode(GT_NE, TYP_INT, temp, exactCls != nullptr ? exactCls : op2); GenTree* condNull; // @@ -11719,7 +11778,12 @@ GenTree* Compiler::impCastClassOrIsInstToTree( // const CorInfoHelpFunc specialHelper = CORINFO_HELP_CHKCASTCLASS_SPECIAL; - condTrue = gtNewHelperCallNode(specialHelper, TYP_REF, gtNewCallArgs(op2Var, gtClone(op1))); + condTrue = + gtNewHelperCallNode(specialHelper, TYP_REF, gtNewCallArgs(partialExpand ? op2 : op2Var, gtClone(op1))); + } + else if (partialExpand) + { + condTrue = gtNewHelperCallNode(helper, TYP_REF, gtNewCallArgs(op2, gtClone(op1))); } else { diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index fe65b551aec98..5114a7c247dab 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -498,7 +498,11 @@ CONFIG_STRING(JitEnablePatchpointRange, W("JitEnablePatchpointRange")) // Profile instrumentation options CONFIG_INTEGER(JitMinimalJitProfiling, W("JitMinimalJitProfiling"), 1) CONFIG_INTEGER(JitMinimalPrejitProfiling, W("JitMinimalPrejitProfiling"), 0) -CONFIG_INTEGER(JitCastProfiling, W("JitCastProfiling"), 0) // Profile castclass and isinst + +CONFIG_INTEGER(JitProfileCasts, W("JitProfileCasts"), 1) // Profile castclass/isinst +CONFIG_INTEGER(JitConsumeProfileForCasts, W("JitConsumeProfileForCasts"), 1) // Consume profile data (if any) for + // castclass/isinst + CONFIG_INTEGER(JitClassProfiling, W("JitClassProfiling"), 1) // Profile virtual and interface calls CONFIG_INTEGER(JitEdgeProfiling, W("JitEdgeProfiling"), 1) // Profile edges instead of blocks CONFIG_INTEGER(JitCollect64BitCounts, W("JitCollect64BitCounts"), 0) // Collect counts as 64-bit values. diff --git a/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.cs b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.cs new file mode 100644 index 0000000000000..3b48f7d462d16 --- /dev/null +++ b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.cs @@ -0,0 +1,214 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Threading; + +public interface IInterface { } +public interface IGenericInterface { } +public class ClassA : IInterface { } +public class ClassB : ClassA { } +public class ClassC { } +public struct GenericStruct : IGenericInterface { } + +public class Program +{ + // 1. Cast to Class + [MethodImpl(MethodImplOptions.NoInlining)] + static ClassA CastToClassA(object o) => (ClassA)o; + + [MethodImpl(MethodImplOptions.NoInlining)] + static GenericStruct CastToGenericStruct(object o) => (GenericStruct)o; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int[] CastToArray(object o) => (int[])o; + + // 2. Is Instance of Class + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IsClassA(object o) => o is ClassA; + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IsGenericStruct(object o) => o is GenericStruct; + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IsArray(object o) => o is int[]; + + // 3. Is Instance of Interface + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IsIInterface(object o) => o is IInterface; + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IsGenericInterface(object o) => o is IGenericInterface; + + // 4. Cast to Interface + + [MethodImpl(MethodImplOptions.NoInlining)] + static IInterface CastToIInterface(object o) => (IInterface)o; + + [MethodImpl(MethodImplOptions.NoInlining)] + static IGenericInterface CastToGenericInterface(object o) => (IGenericInterface)o; + + + [MethodImpl(MethodImplOptions.NoInlining)] + static void AssertThrows(Action action, [CallerLineNumber] int line = 0) where T : Exception + { + try + { + action(); + } + catch (T) + { + return; + } + throw new InvalidOperationException("InvalidCastException was expected"); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool AssertEquals(T t1, T t2) + { + return EqualityComparer.Default.Equals(t1, t2); + } + + public static int Main() + { + var a = new ClassA(); + var b = new ClassB(); + var c = new ClassC(); + var gsInt = new GenericStruct(); + var gsString = new GenericStruct(); + var arrayOfUInt32 = new uint[100]; + var arrayOfInt32 = new int[100]; + var arrayOfString = new string[100]; + + for (int i = 0; i < 200; i++) + { + AssertEquals(IsArray(arrayOfUInt32), true); + AssertEquals(IsArray(arrayOfInt32), true); + AssertEquals(IsArray(arrayOfString), false); + AssertEquals(IsArray(a), false); + AssertEquals(IsArray(b), false); + AssertEquals(IsArray(c), false); + AssertEquals(IsArray(gsInt), false); + AssertEquals(IsArray(gsString), false); + AssertEquals(IsArray(null), false); + + AssertEquals(IsClassA(arrayOfUInt32), false); + AssertEquals(IsClassA(arrayOfInt32), false); + AssertEquals(IsClassA(arrayOfString), false); + AssertEquals(IsClassA(a), true); + AssertEquals(IsClassA(b), false); + AssertEquals(IsClassA(c), false); + AssertEquals(IsClassA(gsInt), false); + AssertEquals(IsClassA(gsString), false); + AssertEquals(IsClassA(null), false); + + AssertEquals(IsGenericStruct(arrayOfUInt32), false); + AssertEquals(IsGenericStruct(arrayOfInt32), false); + AssertEquals(IsGenericStruct(arrayOfString), false); + AssertEquals(IsGenericStruct(a), true); + AssertEquals(IsGenericStruct(b), false); + AssertEquals(IsGenericStruct(c), false); + AssertEquals(IsGenericStruct(gsInt), false); + AssertEquals(IsGenericStruct(gsString), false); + AssertEquals(IsGenericStruct(arrayOfUInt32), false); + AssertEquals(IsGenericStruct(arrayOfInt32), false); + AssertEquals(IsGenericStruct(arrayOfString), false); + AssertEquals(IsGenericStruct(a), true); + AssertEquals(IsGenericStruct(b), false); + AssertEquals(IsGenericStruct(c), false); + AssertEquals(IsGenericStruct(gsInt), false); + AssertEquals(IsGenericStruct(null), false); + + AssertEquals(IsGenericInterface(arrayOfUInt32), false); + AssertEquals(IsGenericInterface(arrayOfInt32), false); + AssertEquals(IsGenericInterface(arrayOfString), false); + AssertEquals(IsGenericInterface(a), true); + AssertEquals(IsGenericInterface(b), false); + AssertEquals(IsGenericInterface(c), false); + AssertEquals(IsGenericInterface(gsInt), false); + AssertEquals(IsGenericInterface(gsString), false); + AssertEquals(IsGenericInterface(arrayOfUInt32), false); + AssertEquals(IsGenericInterface(arrayOfInt32), false); + AssertEquals(IsGenericInterface(arrayOfString), false); + AssertEquals(IsGenericInterface(a), true); + AssertEquals(IsGenericInterface(b), false); + AssertEquals(IsGenericInterface(c), false); + AssertEquals(IsGenericInterface(gsInt), false); + AssertEquals(IsGenericInterface(gsString), false); + AssertEquals(IsGenericInterface(null), false); + + AssertEquals(IsIInterface(arrayOfUInt32), false); + AssertEquals(IsIInterface(arrayOfInt32), false); + AssertEquals(IsIInterface(arrayOfString), false); + AssertEquals(IsIInterface(a), true); + AssertEquals(IsIInterface(b), false); + AssertEquals(IsIInterface(c), false); + AssertEquals(IsIInterface(gsInt), false); + AssertEquals(IsIInterface(gsString), false); + AssertEquals(IsIInterface(null), false); + + AssertThrows(() => CastToClassA(gsInt)); + AssertThrows(() => CastToClassA(gsString)); + AssertThrows(() => CastToClassA(arrayOfUInt32)); + AssertThrows(() => CastToClassA(arrayOfInt32)); + AssertThrows(() => CastToClassA(arrayOfString)); + + AssertThrows(() => CastToArray(a)); + AssertThrows(() => CastToArray(b)); + AssertThrows(() => CastToArray(c)); + AssertThrows(() => CastToArray(gsInt)); + AssertThrows(() => CastToArray(gsString)); + + AssertEquals(CastToIInterface(a), a); + AssertEquals(CastToIInterface(b), b); + AssertThrows(() => CastToIInterface(c)); + AssertThrows(() => CastToIInterface(gsInt)); + AssertThrows(() => CastToIInterface(gsString)); + AssertThrows(() => CastToIInterface(arrayOfUInt32)); + AssertThrows(() => CastToIInterface(arrayOfInt32)); + AssertThrows(() => CastToIInterface(arrayOfString)); + + AssertThrows(() => CastToGenericInterface(a)); + AssertThrows(() => CastToGenericInterface(b)); + AssertThrows(() => CastToGenericInterface(c)); + AssertEquals(CastToGenericInterface(gsInt), gsInt); + AssertThrows(() => CastToGenericInterface(gsString)); + AssertThrows(() => CastToGenericInterface(arrayOfUInt32)); + AssertThrows(() => CastToGenericInterface(arrayOfInt32)); + AssertThrows(() => CastToGenericInterface(arrayOfString)); + AssertThrows(() => CastToGenericInterface(a)); + AssertThrows(() => CastToGenericInterface(b)); + AssertThrows(() => CastToGenericInterface(c)); + AssertThrows(() => CastToGenericInterface(gsInt)); + AssertEquals(CastToGenericInterface(gsString), gsString); + AssertThrows(() => CastToGenericInterface(arrayOfUInt32)); + AssertThrows(() => CastToGenericInterface(arrayOfInt32)); + AssertThrows(() => CastToGenericInterface(arrayOfString)); + + AssertThrows(() => CastToGenericStruct(a)); + AssertThrows(() => CastToGenericStruct(b)); + AssertThrows(() => CastToGenericStruct(c)); + AssertEquals(CastToGenericStruct(gsInt), gsInt); + AssertThrows(() => CastToGenericStruct(gsString)); + AssertThrows(() => CastToGenericStruct(arrayOfUInt32)); + AssertThrows(() => CastToGenericStruct(arrayOfInt32)); + AssertThrows(() => CastToGenericStruct(arrayOfString)); + AssertThrows(() => CastToGenericStruct(a)); + AssertThrows(() => CastToGenericStruct(b)); + AssertThrows(() => CastToGenericStruct(c)); + AssertThrows(() => CastToGenericStruct(gsInt)); + AssertEquals(CastToGenericStruct(gsString), gsString); + AssertThrows(() => CastToGenericStruct(arrayOfUInt32)); + AssertThrows(() => CastToGenericStruct(arrayOfInt32)); + AssertThrows(() => CastToGenericStruct(arrayOfString)); + AssertThrows(() => CastToGenericStruct(null)); + + Thread.Sleep(20); + } + return 100; + } +} diff --git a/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.csproj b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.csproj new file mode 100644 index 0000000000000..f584e2fb22c53 --- /dev/null +++ b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.csproj @@ -0,0 +1,23 @@ + + + Exe + True + + + + + + + \ No newline at end of file diff --git a/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random1.csproj b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random1.csproj new file mode 100644 index 0000000000000..62cbb33acda72 --- /dev/null +++ b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random1.csproj @@ -0,0 +1,25 @@ + + + Exe + True + + + + + + + diff --git a/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random2.csproj b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random2.csproj new file mode 100644 index 0000000000000..b4139f2f1d0b1 --- /dev/null +++ b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random2.csproj @@ -0,0 +1,25 @@ + + + Exe + True + + + + + + + diff --git a/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random3.csproj b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random3.csproj new file mode 100644 index 0000000000000..71e099150b57b --- /dev/null +++ b/src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst_random3.csproj @@ -0,0 +1,25 @@ + + + Exe + True + + + + + + +