Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 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
Expand Up @@ -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
Expand All @@ -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
//
Expand Down
38 changes: 26 additions & 12 deletions src/coreclr/jit/fgbasic.cpp
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::IsConstantOrConstArg(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
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
50 changes: 40 additions & 10 deletions src/coreclr/jit/importer.cpp
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 @@ -4007,6 +4013,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 @@ -5352,6 +5378,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
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
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
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
39 changes: 39 additions & 0 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13223,6 +13223,45 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
}
break;

case GT_INTRINSIC:
if (tree->AsIntrinsic()->gtIntrinsicName ==
NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant)
{
// Should be expanded by the time it reaches CSE phase
assert(!optValnumCSE_phase);

JITDUMP("\nExpanding RuntimeHelpers.IsKnownConstant to ");
if (op1->OperIsConst())
{
// We're lucky to catch a constant here while importer was not
JITDUMP("true\n");
DEBUG_DESTROY_NODE(tree, op1);
tree = gtNewIconNode(1);
}
else
{
GenTree* op1SideEffects = nullptr;
gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT);
if (op1SideEffects != nullptr)
{
DEBUG_DESTROY_NODE(tree);
// Keep side-effects of op1
tree = gtNewOperNode(GT_COMMA, TYP_INT, op1SideEffects, gtNewIconNode(0));
JITDUMP("false with side effects:\n")
DISPTREE(tree);
}
else
{
JITDUMP("false\n");
DEBUG_DESTROY_NODE(tree, op1);
tree = gtNewIconNode(0);
}
}
INDEBUG(tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
return tree;
}
break;

default:
break;
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/namedintrinsiclist.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ enum NamedIntrinsic : unsigned short

NI_System_Runtime_CompilerServices_RuntimeHelpers_CreateSpan,
NI_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray,
NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant,

NI_System_String_get_Chars,
NI_System_String_get_Length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,5 +117,15 @@ internal static bool IsPrimitiveType(this CorElementType et)
/// <remarks>This method is intended for compiler use rather than use directly in code. T must be one of byte, sbyte, char, short, ushort, int, long, ulong, float, or double.</remarks>
[Intrinsic]
public static unsafe ReadOnlySpan<T> CreateSpan<T>(RuntimeFieldHandle fldHandle) => new ReadOnlySpan<T>(GetSpanDataFrom(fldHandle, typeof(T).TypeHandle, out int length), length);


// The following intrinsics return true if input is a compile-time constant
// Feel free to add more overloads on demand

[Intrinsic]
internal static bool IsKnownConstant(string? t) => false;

[Intrinsic]
internal static bool IsKnownConstant(char t) => false;
}
}
Loading