Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rely on PGO for isinst/castclass #65922

Merged
merged 9 commits into from
Apr 1, 2022
Merged

Rely on PGO for isinst/castclass #65922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1d0cb29
Use pgo data for isinst/castclass
EgorBo Feb 26, 2022
1d25f00
Clean up
EgorBo Feb 26, 2022
eaceff1
formatting
EgorBo Feb 26, 2022
6b707f1
Merge branch 'main' of https://github.com/dotnet/runtime into use-pgo…
EgorBo Mar 16, 2022
1d0a723
Add tests
EgorBo Mar 16, 2022
eab07db
Merge branch 'main' of https://github.com/dotnet/runtime into use-pgo…
EgorBo Mar 20, 2022
cf570db
Update ProfileCastClassAndIsInst_random1.csproj
EgorBo Mar 21, 2022
d4a83b3
Update ProfileCastClassAndIsInst_random2.csproj
EgorBo Mar 21, 2022
52c3d04
Update ProfileCastClassAndIsInst_random3.csproj
EgorBo Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4659,7 +4659,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);
Expand Down
92 changes: 78 additions & 14 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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))
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
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;
}
Expand Down Expand Up @@ -11602,7 +11603,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))
Expand All @@ -11615,8 +11616,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.
Expand All @@ -11636,6 +11640,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()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this rather get its own config variable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just didn't want to produce even more variables 😄 and the default value for this one was OK to me.
in my other PR to add "multiple guesses" I slightly changed the whole logic so I'll leave it as is for now

{
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;
Expand Down Expand Up @@ -11687,13 +11746,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;
//
Expand All @@ -11718,7 +11777,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
{
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,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.
Expand Down
214 changes: 214 additions & 0 deletions src/tests/JIT/PGO/ProfileCastClassAndIsInst/ProfileCastClassAndIsInst.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<T> { }
public class ClassA : IInterface { }
public class ClassB : ClassA { }
public class ClassC { }
public struct GenericStruct<T> : IGenericInterface<T> { }

public class Program
{
// 1. Cast to Class
[MethodImpl(MethodImplOptions.NoInlining)]
static ClassA CastToClassA(object o) => (ClassA)o;

[MethodImpl(MethodImplOptions.NoInlining)]
static GenericStruct<T> CastToGenericStruct<T>(object o) => (GenericStruct<T>)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<T>(object o) => o is GenericStruct<T>;

[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<T>(object o) => o is IGenericInterface<T>;

// 4. Cast to Interface

[MethodImpl(MethodImplOptions.NoInlining)]
static IInterface CastToIInterface(object o) => (IInterface)o;

[MethodImpl(MethodImplOptions.NoInlining)]
static IGenericInterface<T> CastToGenericInterface<T>(object o) => (IGenericInterface<T>)o;


[MethodImpl(MethodImplOptions.NoInlining)]
static void AssertThrows<T>(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>(T t1, T t2)
{
return EqualityComparer<T>.Default.Equals(t1, t2);
}

public static int Main()
{
var a = new ClassA();
var b = new ClassB();
var c = new ClassC();
var gsInt = new GenericStruct<int>();
var gsString = new GenericStruct<string>();
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<string>(arrayOfUInt32), false);
AssertEquals(IsGenericStruct<string>(arrayOfInt32), false);
AssertEquals(IsGenericStruct<string>(arrayOfString), false);
AssertEquals(IsGenericStruct<string>(a), true);
AssertEquals(IsGenericStruct<string>(b), false);
AssertEquals(IsGenericStruct<string>(c), false);
AssertEquals(IsGenericStruct<string>(gsInt), false);
AssertEquals(IsGenericStruct<string>(gsString), false);
AssertEquals(IsGenericStruct<int>(arrayOfUInt32), false);
AssertEquals(IsGenericStruct<int>(arrayOfInt32), false);
AssertEquals(IsGenericStruct<int>(arrayOfString), false);
AssertEquals(IsGenericStruct<int>(a), true);
AssertEquals(IsGenericStruct<int>(b), false);
AssertEquals(IsGenericStruct<int>(c), false);
AssertEquals(IsGenericStruct<int>(gsInt), false);
AssertEquals(IsGenericStruct<int>(null), false);

AssertEquals(IsGenericInterface<string>(arrayOfUInt32), false);
AssertEquals(IsGenericInterface<string>(arrayOfInt32), false);
AssertEquals(IsGenericInterface<string>(arrayOfString), false);
AssertEquals(IsGenericInterface<string>(a), true);
AssertEquals(IsGenericInterface<string>(b), false);
AssertEquals(IsGenericInterface<string>(c), false);
AssertEquals(IsGenericInterface<string>(gsInt), false);
AssertEquals(IsGenericInterface<string>(gsString), false);
AssertEquals(IsGenericInterface<int>(arrayOfUInt32), false);
AssertEquals(IsGenericInterface<int>(arrayOfInt32), false);
AssertEquals(IsGenericInterface<int>(arrayOfString), false);
AssertEquals(IsGenericInterface<int>(a), true);
AssertEquals(IsGenericInterface<int>(b), false);
AssertEquals(IsGenericInterface<int>(c), false);
AssertEquals(IsGenericInterface<int>(gsInt), false);
AssertEquals(IsGenericInterface<int>(gsString), false);
AssertEquals(IsGenericInterface<int>(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<InvalidCastException>(() => CastToClassA(gsInt));
AssertThrows<InvalidCastException>(() => CastToClassA(gsString));
AssertThrows<InvalidCastException>(() => CastToClassA(arrayOfUInt32));
AssertThrows<InvalidCastException>(() => CastToClassA(arrayOfInt32));
AssertThrows<InvalidCastException>(() => CastToClassA(arrayOfString));

AssertThrows<InvalidCastException>(() => CastToArray(a));
AssertThrows<InvalidCastException>(() => CastToArray(b));
AssertThrows<InvalidCastException>(() => CastToArray(c));
AssertThrows<InvalidCastException>(() => CastToArray(gsInt));
AssertThrows<InvalidCastException>(() => CastToArray(gsString));

AssertEquals(CastToIInterface(a), a);
AssertEquals(CastToIInterface(b), b);
AssertThrows<InvalidCastException>(() => CastToIInterface(c));
AssertThrows<InvalidCastException>(() => CastToIInterface(gsInt));
AssertThrows<InvalidCastException>(() => CastToIInterface(gsString));
AssertThrows<InvalidCastException>(() => CastToIInterface(arrayOfUInt32));
AssertThrows<InvalidCastException>(() => CastToIInterface(arrayOfInt32));
AssertThrows<InvalidCastException>(() => CastToIInterface(arrayOfString));

AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(a));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(b));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(c));
AssertEquals(CastToGenericInterface<int>(gsInt), gsInt);
AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(gsString));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(arrayOfUInt32));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(arrayOfInt32));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<int>(arrayOfString));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(a));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(b));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(c));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(gsInt));
AssertEquals(CastToGenericInterface<string>(gsString), gsString);
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(arrayOfUInt32));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(arrayOfInt32));
AssertThrows<InvalidCastException>(() => CastToGenericInterface<string>(arrayOfString));

AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(a));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(b));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(c));
AssertEquals(CastToGenericStruct<int>(gsInt), gsInt);
AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(gsString));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(arrayOfUInt32));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(arrayOfInt32));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<int>(arrayOfString));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(a));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(b));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(c));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(gsInt));
AssertEquals(CastToGenericStruct<string>(gsString), gsString);
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(arrayOfUInt32));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(arrayOfInt32));
AssertThrows<InvalidCastException>(() => CastToGenericStruct<string>(arrayOfString));
AssertThrows<NullReferenceException>(() => CastToGenericStruct<string>(null));

Thread.Sleep(20);
}
return 100;
}
}
Loading