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.

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

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

Merged
merged 26 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
6 changes: 6 additions & 0 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,12 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
break;
}

case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
pushedStack.PushConstant();
foldableIntrinsc = true;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
// we can add an additional boost if arg is really a const
break;

// These are foldable if the first argument is a constant
case NI_System_Type_get_IsValueType:
case NI_System_Type_GetTypeFromHandle:
Expand Down
48 changes: 38 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,24 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
break;
}

case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
{
GenTree* op1 = impPopStack().val;
if (op1->OperIsConst() || gtFoldExpr(op1)->OperIsConst())
{
// op1 is a known constant, replace with 'true'.
retNode = gtNewIconNode(1);
// We can also consider FTN_ADDR and typeof(T) here
}
else
{
// op1 is not a known constant, replace with 'false'.
// TODO: delay till e.g. morph and try again, maybe op1 will become a const.
retNode = gtNewIconNode(0);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}
break;
}

case NI_System_Activator_AllocatorOf:
case NI_System_Activator_DefaultConstructorOf:
case NI_System_Object_MethodTableOf:
Expand Down Expand Up @@ -5352,6 +5376,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
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,11 @@ 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);

/// <summary>
/// Returns true if input is a compile-time constant
/// </summary>
[Intrinsic]
internal static bool IsKnownConstant<T>(T t) => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,21 @@ public bool Equals([NotNullWhen(true)] string? value, StringComparison compariso
}

// Determines whether two Strings match.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool Equals(string? a, string? b)
{
// if either a or b are "" - optimize Equals to just 'str?.Length == 0'
// Otherwise, these two blocks are eliminated since IsKnownConstant is a jit-time constant
if (RuntimeHelpers.IsKnownConstant(a) && a?.Length == 0)
{
return b?.Length == 0;
}

if (RuntimeHelpers.IsKnownConstant(b) && b?.Length == 0)
{
return a?.Length == 0;
}

if (object.ReferenceEquals(a, b))
{
return true;
Expand Down Expand Up @@ -1013,7 +1026,15 @@ public bool StartsWith(string value, bool ignoreCase, CultureInfo? culture)
return referenceCulture.CompareInfo.IsPrefix(this, value, ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None);
}

public bool StartsWith(char value) => Length != 0 && _firstChar == value;
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool StartsWith(char value)
{
if (RuntimeHelpers.IsKnownConstant(value) && value != '\0')
{
return _firstChar == value;
}
return Length != 0 && _firstChar == value;
}

internal static void CheckStringComparison(StringComparison comparisonType)
{
Expand Down