From 51a18cef2440a10db5526c80f35bc90027677ead Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 4 Feb 2019 15:46:26 +0000 Subject: [PATCH 1/3] Fix strlen --- .../shared/System/SpanHelpers.Byte.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs index 443c7bed9e74..137e985c6e7d 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs @@ -268,6 +268,29 @@ public static unsafe int IndexOf(ref byte searchSpace, byte value, int length) if (Avx2.IsSupported) { + if ((((nuint)Unsafe.AsPointer(ref searchSpace) + (nuint)offset) & (nuint)(Vector256.Count - 1)) != 0) + { + // Not currently aligned to Vector256 (is aligned to Vector128); this can cause a problem for searches + // with no upper bound e.g. String.strlen. + // Start with a check on Vector128 to align to Vector256, before moving to processing Vector256. + // This ensures we do not fault across memory pages while searching for an end of string. + Vector128 values = Vector128.Create(value); + Vector128 search = LoadVector128(ref searchSpace, offset); + + // Same method as below + int matches = Sse2.MoveMask(Sse2.CompareEqual(values, search)); + if (matches == 0) + { + // Zero flags set so no matches + offset += Vector128.Count; + } + else + { + // Find bitflag offset of first match and add to current offset + return ((int)(byte*)offset) + BitOps.TrailingZeroCount(matches); + } + } + if ((int)(byte*)offset < length) { nLength = GetByteVector256SpanLength(offset, length); From cd13d3dc8c0ca4df9476c90e5c9e5d7ddee70c91 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 4 Feb 2019 21:37:29 +0000 Subject: [PATCH 2/3] Vector should be in conditonal --- .../shared/System/SpanHelpers.Byte.cs | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs index 137e985c6e7d..4039da3a74b0 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs @@ -268,31 +268,31 @@ public static unsafe int IndexOf(ref byte searchSpace, byte value, int length) if (Avx2.IsSupported) { - if ((((nuint)Unsafe.AsPointer(ref searchSpace) + (nuint)offset) & (nuint)(Vector256.Count - 1)) != 0) + if ((int)(byte*)offset < length) { - // Not currently aligned to Vector256 (is aligned to Vector128); this can cause a problem for searches - // with no upper bound e.g. String.strlen. - // Start with a check on Vector128 to align to Vector256, before moving to processing Vector256. - // This ensures we do not fault across memory pages while searching for an end of string. - Vector128 values = Vector128.Create(value); - Vector128 search = LoadVector128(ref searchSpace, offset); - - // Same method as below - int matches = Sse2.MoveMask(Sse2.CompareEqual(values, search)); - if (matches == 0) + if ((((nuint)Unsafe.AsPointer(ref searchSpace) + (nuint)offset) & (nuint)(Vector256.Count - 1)) != 0) { - // Zero flags set so no matches - offset += Vector128.Count; - } - else - { - // Find bitflag offset of first match and add to current offset - return ((int)(byte*)offset) + BitOps.TrailingZeroCount(matches); + // Not currently aligned to Vector256 (is aligned to Vector128); this can cause a problem for searches + // with no upper bound e.g. String.strlen. + // Start with a check on Vector128 to align to Vector256, before moving to processing Vector256. + // This ensures we do not fault across memory pages while searching for an end of string. + Vector128 values = Vector128.Create(value); + Vector128 search = LoadVector128(ref searchSpace, offset); + + // Same method as below + int matches = Sse2.MoveMask(Sse2.CompareEqual(values, search)); + if (matches == 0) + { + // Zero flags set so no matches + offset += Vector128.Count; + } + else + { + // Find bitflag offset of first match and add to current offset + return ((int)(byte*)offset) + BitOps.TrailingZeroCount(matches); + } } - } - if ((int)(byte*)offset < length) - { nLength = GetByteVector256SpanLength(offset, length); if ((byte*)nLength > (byte*)offset) { From 1022716b35074b88509b0bef0395069b232becbf Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 4 Feb 2019 23:18:12 +0000 Subject: [PATCH 3/3] Add explaination comment --- src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs index 4039da3a74b0..9672f343ebcd 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs @@ -266,6 +266,9 @@ public static unsafe int IndexOf(ref byte searchSpace, byte value, int length) offset += 1; } + // We get past SequentialScan only if IsHardwareAccelerated or intrinsic .IsSupported is true; and remain length is greater than Vector length. + // However, we still have the redundant check to allow the JIT to see that the code is unreachable and eliminate it when the platform does not + // have hardware accelerated. After processing Vector lengths we return to SequentialScan to finish any remaining. if (Avx2.IsSupported) { if ((int)(byte*)offset < length)