Skip to content

Commit

Permalink
Add IsKnownConstant jit helper and optimize 'str == ""' with str.Star…
Browse files Browse the repository at this point in the history 
…tsWith('c') (#63734)

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
  • Loading branch information
@EgorBo @MihaZupan @SingleAccretion
3 people authored Jan 19, 2022
1 parent ded841a commit 65db54e
Show file tree
Hide file tree
Showing 13 changed files with 305 additions and 51 deletions.
24 changes: 19 additions & 5 deletions src/coreclr/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4516,11 +4516,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
Expand All @@ -4541,6 +4542,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
//
Expand Down
38 changes: 26 additions & 12 deletions src/coreclr/jit/fgbasic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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;
}

Expand All @@ -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:
Expand All @@ -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;
}
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10824,6 +10824,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();
Expand Down
82 changes: 51 additions & 31 deletions src/coreclr/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/inline.def
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 21 additions & 1 deletion src/coreclr/jit/inlinepolicy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/inlinepolicy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ class DefaultPolicy : public LegalPolicy
, m_CallsiteIsInLoop(false)
, m_IsNoReturn(false)
, m_IsNoReturnKnown(false)
, m_ConstArgFeedsIsKnownConst(false)
, m_ArgFeedsIsKnownConst(false)
{
// empty
}
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 65db54e

Please sign in to comment.