Skip to content

Add IsKnownConstant jit helper and optimize 'str == ""' with str.StartsWith('c') #63734

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.

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

Merged
merged 26 commits into from
Jan 19, 2022
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9f55fa6
Don't reuse args in EnC
EgorBo Jan 12, 2022
7d3ef94
Merge branch 'main' of https://github.com/dotnet/runtime into fix-enc…
EgorBo Jan 13, 2022
0a717aa
Introduce RuntimeHelpers.IsKnownConstant and use for 'str == ""'
EgorBo Jan 13, 2022
407a953
Remove unrelated change, add gtFoldExpr
EgorBo Jan 13, 2022
9d45461
Also, optimize String.StartsWith
EgorBo Jan 13, 2022
2f8a034
Update comment
EgorBo Jan 13, 2022
4883a46
Update src/libraries/System.Private.CoreLib/src/System/String.Compari…
EgorBo Jan 13, 2022
aa88cf9
Delay expansion to morph, replace generic signature with overloads
EgorBo Jan 13, 2022
64843f0
Merge branch 'jit-isknownconstant' of https://github.com/EgorBo/runti…
EgorBo Jan 13, 2022
2ebdae9
Make string overload nullable
EgorBo Jan 13, 2022
e264d27
Use GTF_ALL_EFFECT
EgorBo Jan 13, 2022
f08a182
Address feedback
EgorBo Jan 13, 2022
8e5fd0d
fix copy-paste
EgorBo Jan 13, 2022
e2ebf30
Update src/coreclr/jit/compiler.hpp
EgorBo Jan 13, 2022
895abc4
Update src/coreclr/jit/compiler.hpp
EgorBo Jan 13, 2022
45bfe7c
Address feedback
EgorBo Jan 13, 2022
80f9ddd
Remove [MethodImpl(MethodImplOptions.AggressiveInlining)], handle it …
EgorBo Jan 13, 2022
eebd8d1
clean up
EgorBo Jan 13, 2022
e0e3e51
Only boost for constant-arg cases
EgorBo Jan 13, 2022
5b2fbfe
Remove redundant opts.OptimizationEnabled()s
EgorBo Jan 13, 2022
30e1855
Update importer.cpp
EgorBo Jan 14, 2022
3cb8402
Update String.Comparison.cs
EgorBo Jan 14, 2022
7c7c264
Update String.Comparison.cs
EgorBo Jan 14, 2022
a9b85fc
Add tests
EgorBo Jan 14, 2022
a3e5f3c
Merge branch 'jit-isknownconstant' of https://github.com/EgorBo/runti…
EgorBo Jan 14, 2022
4947947
Fix unrelated invalid IR, we should not import TYP_USHORT constant
EgorBo Jan 14, 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
24 changes: 19 additions & 5 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
@@ -4504,11 +4504,12 @@ inline static bool StructHasNoPromotionFlagSet(DWORD attribs)
return ((attribs & CORINFO_FLG_DONT_PROMOTE) != 0);
}

/*****************************************************************************
* This node should not be referenced by anyone now. Set its values to garbage
* to catch extra references
*/

//------------------------------------------------------------------------------
// DEBUG_DESTROY_NODE: sets value of tree to garbage to catch extra references
//
// Arguments:
// tree: This node should not be referenced by anyone now
//
inline void DEBUG_DESTROY_NODE(GenTree* tree)
{
#ifdef DEBUG
@@ -4529,6 +4530,19 @@ inline void DEBUG_DESTROY_NODE(GenTree* tree)
#endif
}

//------------------------------------------------------------------------------
// DEBUG_DESTROY_NODE: sets value of trees to garbage to catch extra references
//
// Arguments:
// tree, ...rest: These nodes should not be referenced by anyone now
//
template <typename... T>
void DEBUG_DESTROY_NODE(GenTree* tree, T... rest)
{
DEBUG_DESTROY_NODE(tree);
DEBUG_DESTROY_NODE(rest...);
}

//------------------------------------------------------------------------------
// lvRefCnt: access reference count for this local var
//
38 changes: 26 additions & 12 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
@@ -1113,12 +1113,12 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
{
ni = lookupNamedIntrinsic(methodHnd);

bool foldableIntrinsc = false;
bool foldableIntrinsic = false;

if (IsMathIntrinsic(ni))
{
// Most Math(F) intrinsics have single arguments
foldableIntrinsc = FgStack::IsConstantOrConstArg(pushedStack.Top(), impInlineInfo);
foldableIntrinsic = FgStack::IsConstantOrConstArg(pushedStack.Top(), impInlineInfo);
}
else
{
@@ -1131,7 +1131,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
case NI_System_GC_KeepAlive:
{
pushedStack.PushUnknown();
foldableIntrinsc = true;
foldableIntrinsic = true;
break;
}

@@ -1145,6 +1145,20 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
break;
}

case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
if (FgStack::IsConstArgument(pushedStack.Top(), impInlineInfo))
{
compInlineResult->Note(InlineObservation::CALLEE_CONST_ARG_FEEDS_ISCONST);
}
else
{
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_ISCONST);
}
// RuntimeHelpers.IsKnownConstant is always folded into a const
pushedStack.PushConstant();
foldableIntrinsic = true;
break;

// These are foldable if the first argument is a constant
case NI_System_Type_get_IsValueType:
case NI_System_Type_GetTypeFromHandle:
@@ -1159,10 +1173,10 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
case NI_Vector128_Create:
#endif
{
// Top() in order to keep it as is in case of foldableIntrinsc
// Top() in order to keep it as is in case of foldableIntrinsic
if (FgStack::IsConstantOrConstArg(pushedStack.Top(), impInlineInfo))
{
foldableIntrinsc = true;
foldableIntrinsic = true;
}
break;
}
@@ -1177,7 +1191,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
if (FgStack::IsConstantOrConstArg(pushedStack.Top(0), impInlineInfo) &&
FgStack::IsConstantOrConstArg(pushedStack.Top(1), impInlineInfo))
{
foldableIntrinsc = true;
foldableIntrinsic = true;
pushedStack.PushConstant();
}
break;
@@ -1186,31 +1200,31 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
case NI_IsSupported_True:
case NI_IsSupported_False:
{
foldableIntrinsc = true;
foldableIntrinsic = true;
pushedStack.PushConstant();
break;
}
#if defined(TARGET_XARCH) && defined(FEATURE_HW_INTRINSICS)
case NI_Vector128_get_Count:
case NI_Vector256_get_Count:
foldableIntrinsc = true;
foldableIntrinsic = true;
pushedStack.PushConstant();
// TODO: check if it's a loop condition - we unroll such loops.
break;
case NI_Vector256_get_Zero:
case NI_Vector256_get_AllBitsSet:
foldableIntrinsc = true;
foldableIntrinsic = true;
pushedStack.PushUnknown();
break;
#elif defined(TARGET_ARM64) && defined(FEATURE_HW_INTRINSICS)
case NI_Vector64_get_Count:
case NI_Vector128_get_Count:
foldableIntrinsc = true;
foldableIntrinsic = true;
pushedStack.PushConstant();
break;
case NI_Vector128_get_Zero:
case NI_Vector128_get_AllBitsSet:
foldableIntrinsc = true;
foldableIntrinsic = true;
pushedStack.PushUnknown();
break;
#endif
@@ -1222,7 +1236,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
}
}

if (foldableIntrinsc)
if (foldableIntrinsic)
{
compInlineResult->Note(InlineObservation::CALLSITE_FOLDABLE_INTRINSIC);
handled = true;
3 changes: 3 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
@@ -10827,6 +10827,9 @@ void Compiler::gtDispTree(GenTree* tree,
case NI_System_Object_GetType:
printf(" objGetType");
break;
case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
printf(" isKnownConst");
break;

default:
unreached();
82 changes: 51 additions & 31 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
@@ -3879,19 +3879,25 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
return new (this, GT_LABEL) GenTree(GT_LABEL, TYP_I_IMPL);
}

if (((ni == NI_System_Runtime_CompilerServices_RuntimeHelpers_CreateSpan) ||
(ni == NI_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray)) &&
IsTargetAbi(CORINFO_CORERT_ABI))
switch (ni)
{
// CreateSpan must be expanded for NativeAOT
mustExpand = true;
}
case NI_System_Runtime_CompilerServices_RuntimeHelpers_CreateSpan:
case NI_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray:
mustExpand |= IsTargetAbi(CORINFO_CORERT_ABI);
break;

if ((ni == NI_System_ByReference_ctor) || (ni == NI_System_ByReference_get_Value) ||
(ni == NI_System_Activator_AllocatorOf) || (ni == NI_System_Activator_DefaultConstructorOf) ||
(ni == NI_System_Object_MethodTableOf) || (ni == NI_System_EETypePtr_EETypePtrOf))
{
mustExpand = true;
case NI_System_ByReference_ctor:
case NI_System_ByReference_get_Value:
case NI_System_Activator_AllocatorOf:
case NI_System_Activator_DefaultConstructorOf:
case NI_System_Object_MethodTableOf:
case NI_System_EETypePtr_EETypePtrOf:
mustExpand = true;
break;

default:
break;
}

GenTree* retNode = nullptr;
@@ -3935,29 +3941,19 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
case NI_System_String_get_Length:
{
GenTree* op1 = impPopStack().val;
if (opts.OptimizationEnabled())
if (op1->OperIs(GT_CNS_STR))
{
if (op1->OperIs(GT_CNS_STR))
// Optimize `ldstr + String::get_Length()` to CNS_INT
// e.g. "Hello".Length => 5
GenTreeIntCon* iconNode = gtNewStringLiteralLength(op1->AsStrCon());
if (iconNode != nullptr)
{
// Optimize `ldstr + String::get_Length()` to CNS_INT
// e.g. "Hello".Length => 5
GenTreeIntCon* iconNode = gtNewStringLiteralLength(op1->AsStrCon());
if (iconNode != nullptr)
{
retNode = iconNode;
break;
}
retNode = iconNode;
break;
}
GenTreeArrLen* arrLen = gtNewArrLen(TYP_INT, op1, OFFSETOF__CORINFO_String__stringLen, compCurBB);
op1 = arrLen;
}
else
{
/* Create the expression "*(str_addr + stringLengthOffset)" */
op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1,
gtNewIconNode(OFFSETOF__CORINFO_String__stringLen, TYP_I_IMPL));
op1 = gtNewOperNode(GT_IND, TYP_INT, op1);
}
GenTreeArrLen* arrLen = gtNewArrLen(TYP_INT, op1, OFFSETOF__CORINFO_String__stringLen, compCurBB);
op1 = arrLen;

// Getting the length of a null string should throw
op1->gtFlags |= GTF_EXCEPT;
@@ -4007,6 +4003,26 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
break;
}

case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
{
GenTree* op1 = impPopStack().val;
if (op1->OperIsConst())
{
// op1 is a known constant, replace with 'true'.
retNode = gtNewIconNode(1);
JITDUMP("\nExpanding RuntimeHelpers.IsKnownConstant to true early\n");
// We can also consider FTN_ADDR and typeof(T) here
}
else
{
// op1 is not a known constant, we'll do the expansion in morph
retNode = new (this, GT_INTRINSIC) GenTreeIntrinsic(TYP_INT, op1, ni, method);
JITDUMP("\nConverting RuntimeHelpers.IsKnownConstant to:\n");
DISPTREE(retNode);
}
break;
}

case NI_System_Activator_AllocatorOf:
case NI_System_Activator_DefaultConstructorOf:
case NI_System_Object_MethodTableOf:
@@ -4283,7 +4299,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,

case NI_System_Threading_Thread_get_ManagedThreadId:
{
if (opts.OptimizationEnabled() && impStackTop().val->OperIs(GT_RET_EXPR))
if (impStackTop().val->OperIs(GT_RET_EXPR))
{
GenTreeCall* call = impStackTop().val->AsRetExpr()->gtInlineCandidate->AsCall();
if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
@@ -4306,7 +4322,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
case NI_System_Threading_Interlocked_Or:
case NI_System_Threading_Interlocked_And:
{
if (opts.OptimizationEnabled() && compOpportunisticallyDependsOn(InstructionSet_Atomics))
if (compOpportunisticallyDependsOn(InstructionSet_Atomics))
{
assert(sig->numArgs == 2);
GenTree* op2 = impPopStack().val;
@@ -5352,6 +5368,10 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
{
result = NI_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray;
}
else if (strcmp(methodName, "IsKnownConstant") == 0)
{
result = NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant;
}
}
else if (strncmp(namespaceName, "System.Runtime.Intrinsics", 25) == 0)
{
2 changes: 2 additions & 0 deletions src/coreclr/jit/inline.def
Original file line number Diff line number Diff line change
@@ -70,6 +70,8 @@ INLINE_OBSERVATION(ARG_FEEDS_CONSTANT_TEST, bool, "argument feeds constant t
INLINE_OBSERVATION(ARG_FEEDS_TEST, bool, "argument feeds test", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_CAST, int, "argument feeds castclass or isinst", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_RANGE_CHECK, bool, "argument feeds range check", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_ISCONST, bool, "argument feeds IsKnownConstant", INFORMATION, CALLEE)
INLINE_OBSERVATION(CONST_ARG_FEEDS_ISCONST, bool, "const argument feeds IsKnownConstant", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_STRUCT, int, "arg is a struct passed by value", INFORMATION, CALLEE)
INLINE_OBSERVATION(RETURNS_STRUCT, bool, "returns a struct by value", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_STRUCT_FIELD_ACCESS, int, "ldfld/stfld over arg (struct)", INFORMATION, CALLEE)
22 changes: 21 additions & 1 deletion src/coreclr/jit/inlinepolicy.cpp
Original file line number Diff line number Diff line change
@@ -326,6 +326,14 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
m_ArgFeedsRangeCheck++;
break;

case InlineObservation::CALLEE_CONST_ARG_FEEDS_ISCONST:
m_ConstArgFeedsIsKnownConst = true;
break;

case InlineObservation::CALLEE_ARG_FEEDS_ISCONST:
m_ArgFeedsIsKnownConst = true;
break;

case InlineObservation::CALLEE_UNSUPPORTED_OPCODE:
propagate = true;
break;
@@ -727,6 +735,15 @@ double DefaultPolicy::DetermineMultiplier()
JITDUMP("\nInline candidate has arg that feeds range check. Multiplier increased to %g.", multiplier);
}

if (m_ConstArgFeedsIsKnownConst || (m_ArgFeedsIsKnownConst && m_IsPrejitRoot))
{
// if we use RuntimeHelpers.IsKnownConstant we most likely expect our function to be always inlined
// at least in the case of constant arguments. In IsPrejitRoot we don't have callsite info so let's
// assume we have a constant here in order to avoid "baked" noinline
multiplier += 20;
JITDUMP("\nConstant argument feeds RuntimeHelpers.IsKnownConstant. Multiplier increased to %g.", multiplier);
}

if (m_ConstantArgFeedsConstantTest > 0)
{
multiplier += 3.0;
@@ -1371,7 +1388,7 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
{
SetNever(InlineObservation::CALLEE_DOES_NOT_RETURN);
}
else if (!m_IsForceInline && !m_HasProfile)
else if (!m_IsForceInline && !m_HasProfile && !m_ConstArgFeedsIsKnownConst && !m_ArgFeedsIsKnownConst)
{
unsigned bbLimit = (unsigned)JitConfig.JitExtDefaultPolicyMaxBB();
if (m_IsPrejitRoot)
@@ -1381,6 +1398,7 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
bbLimit += 5 + m_Switch * 10;
}
bbLimit += m_FoldableBranch + m_FoldableSwitch * 10;

if ((unsigned)value > bbLimit)
{
SetNever(InlineObservation::CALLEE_TOO_MANY_BASIC_BLOCKS);
@@ -2638,6 +2656,8 @@ void DiscretionaryPolicy::DumpData(FILE* file) const
fprintf(file, ",%u", m_ArgFeedsConstantTest);
fprintf(file, ",%u", m_MethodIsMostlyLoadStore ? 1 : 0);
fprintf(file, ",%u", m_ArgFeedsRangeCheck);
fprintf(file, ",%u", m_ConstArgFeedsIsKnownConst ? 1 : 0);
fprintf(file, ",%u", m_ArgFeedsIsKnownConst ? 1 : 0);
fprintf(file, ",%u", m_ConstantArgFeedsConstantTest);
fprintf(file, ",%d", m_CalleeNativeSizeEstimate);
fprintf(file, ",%d", m_CallsiteNativeSizeEstimate);
4 changes: 4 additions & 0 deletions src/coreclr/jit/inlinepolicy.h
Original file line number Diff line number Diff line change
@@ -110,6 +110,8 @@ class DefaultPolicy : public LegalPolicy
, m_CallsiteIsInLoop(false)
, m_IsNoReturn(false)
, m_IsNoReturnKnown(false)
, m_ConstArgFeedsIsKnownConst(false)
, m_ArgFeedsIsKnownConst(false)
{
// empty
}
@@ -178,6 +180,8 @@ class DefaultPolicy : public LegalPolicy
bool m_CallsiteIsInLoop : 1;
bool m_IsNoReturn : 1;
bool m_IsNoReturnKnown : 1;
bool m_ConstArgFeedsIsKnownConst : 1;
bool m_ArgFeedsIsKnownConst : 1;
};

// ExtendedDefaultPolicy is a slightly more aggressive variant of
Loading