From 47eb0eb853c0ae80e47eb556264bf2008e54a7c2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 8 May 2023 17:15:54 +0200 Subject: [PATCH] JIT: Boost inlining if we can unroll memmove/memcmp --- src/coreclr/jit/fgbasic.cpp | 14 ++++++++++++++ src/coreclr/jit/importercalls.cpp | 8 ++++++-- src/coreclr/jit/inline.def | 1 + src/coreclr/jit/inlinepolicy.cpp | 14 +++++++++++++- src/coreclr/jit/inlinepolicy.h | 2 ++ .../System.Private.CoreLib/src/System/Buffer.cs | 1 + .../src/System/Text/StringBuilder.cs | 4 ++-- 7 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 7b19cfd06c915..0e009919acbd4 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -1125,6 +1125,20 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed break; } + case NI_System_SpanHelpers_SequenceEqual: + case NI_System_Buffer_Memmove: + { + if (FgStack::IsConstArgument(pushedStack.Top(), impInlineInfo)) + { + // Constant (at its call-site) argument feeds the Memmove/Memcmp length argument. + // We most likely will be able to unroll it. + // It is important to only raise this hint for constant arguments, if it's just a + // constant in the inlinee itself then we don't need to inline it for unrolling. + compInlineResult->Note(InlineObservation::CALLSITE_UNROLLABLE_MEMOP); + } + break; + } + case NI_System_Span_get_Item: case NI_System_ReadOnlySpan_get_Item: { diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index ce7d2f286db33..b042704862e25 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3795,8 +3795,12 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_SpanHelpers_SequenceEqual: case NI_System_Buffer_Memmove: { - // We'll try to unroll this in lower for constant input. - isSpecial = true; + if (sig->sigInst.methInstCount == 0) + { + // We'll try to unroll this in lower for constant input. + isSpecial = true; + } + // The generic version is also marked as [Intrinsic] just as a hint for the inliner break; } diff --git a/src/coreclr/jit/inline.def b/src/coreclr/jit/inline.def index 06b6ae681b9e1..b918fb61282dd 100644 --- a/src/coreclr/jit/inline.def +++ b/src/coreclr/jit/inline.def @@ -180,6 +180,7 @@ INLINE_OBSERVATION(FOLDABLE_EXPR, int, "foldable binary expressio INLINE_OBSERVATION(FOLDABLE_EXPR_UN, int, "foldable unary expression", INFORMATION, CALLSITE) INLINE_OBSERVATION(FOLDABLE_BRANCH, int, "foldable branch", INFORMATION, CALLSITE) INLINE_OBSERVATION(FOLDABLE_SWITCH, int, "foldable switch", INFORMATION, CALLSITE) +INLINE_OBSERVATION(UNROLLABLE_MEMOP, int, "unrollable memmove/memcmp", INFORMATION, CALLSITE) INLINE_OBSERVATION(DIV_BY_CNS, int, "dividy by const", INFORMATION, CALLSITE) INLINE_OBSERVATION(CONSTANT_ARG_FEEDS_TEST, bool, "constant argument feeds test", INFORMATION, CALLSITE) INLINE_OBSERVATION(DEPTH, int, "depth", INFORMATION, CALLSITE) diff --git a/src/coreclr/jit/inlinepolicy.cpp b/src/coreclr/jit/inlinepolicy.cpp index ebba45105bbd1..3d1d895c434a5 100644 --- a/src/coreclr/jit/inlinepolicy.cpp +++ b/src/coreclr/jit/inlinepolicy.cpp @@ -1317,6 +1317,10 @@ void ExtendedDefaultPolicy::NoteBool(InlineObservation obs, bool value) m_FoldableSwitch++; break; + case InlineObservation::CALLSITE_UNROLLABLE_MEMOP: + m_UnrollableMemop++; + break; + case InlineObservation::CALLEE_HAS_SWITCH: m_Switch++; break; @@ -1409,7 +1413,7 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value) // in prejit-root mode. bbLimit += 5 + m_Switch * 10; } - bbLimit += m_FoldableBranch + m_FoldableSwitch * 10; + bbLimit += m_FoldableBranch + m_FoldableSwitch * 10 + m_UnrollableMemop * 2; if ((unsigned)value > bbLimit) { @@ -1715,6 +1719,13 @@ double ExtendedDefaultPolicy::DetermineMultiplier() break; } + if (m_UnrollableMemop > 0) + { + multiplier += m_UnrollableMemop; + JITDUMP("\nInline candidate has %d unrollable memory operations. Multiplier increased to %g.", + m_UnrollableMemop, multiplier); + } + if (m_FoldableSwitch > 0) { multiplier += 6.0; @@ -1841,6 +1852,7 @@ void ExtendedDefaultPolicy::OnDumpXml(FILE* file, unsigned indent) const XATTR_I4(m_FoldableExprUn) XATTR_I4(m_FoldableBranch) XATTR_I4(m_FoldableSwitch) + XATTR_I4(m_UnrollableMemop) XATTR_I4(m_Switch) XATTR_I4(m_DivByCns) XATTR_B(m_ReturnsStructByValue) diff --git a/src/coreclr/jit/inlinepolicy.h b/src/coreclr/jit/inlinepolicy.h index cc15348567d6d..57c171f3e99c8 100644 --- a/src/coreclr/jit/inlinepolicy.h +++ b/src/coreclr/jit/inlinepolicy.h @@ -216,6 +216,7 @@ class ExtendedDefaultPolicy : public DefaultPolicy , m_FoldableExprUn(0) , m_FoldableBranch(0) , m_FoldableSwitch(0) + , m_UnrollableMemop(0) , m_Switch(0) , m_DivByCns(0) , m_ReturnsStructByValue(false) @@ -268,6 +269,7 @@ class ExtendedDefaultPolicy : public DefaultPolicy unsigned m_FoldableExprUn; unsigned m_FoldableBranch; unsigned m_FoldableSwitch; + unsigned m_UnrollableMemop; unsigned m_Switch; unsigned m_DivByCns; bool m_ReturnsStructByValue : 1; diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffer.cs b/src/libraries/System.Private.CoreLib/src/System/Buffer.cs index 8328c18610378..f343c6e4e8b0b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffer.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffer.cs @@ -345,6 +345,7 @@ internal static unsafe void _ZeroMemory(ref byte b, nuint byteLength) #if !MONO // Mono BulkMoveWithWriteBarrier is in terms of elements (not bytes) and takes a type handle. + [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static unsafe void Memmove(ref T destination, ref T source, nuint elementCount) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs index e2ea2a6b5d145..e0d818c8439f0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs @@ -728,9 +728,9 @@ public StringBuilder Append(char[]? value, int startIndex, int charCount) /// The string to append. public StringBuilder Append(string? value) { - if (value is not null) + if (value != null) { - Append(valueCount: value.Length, value: ref value.GetRawStringData()); + Append(ref value.GetRawStringData(), value.Length); } return this;