Intrinsicify SpanHelpers.IndexOf(char)

[benaadams]

Based on #22875 which is first commit

and Vectorize wcslen

From #22187

 -------------------- |--------- |-----------:|
-      String_IndexOf |        7 |   5.031 ns |
+      String_IndexOf |        7 |   4.403 ns |
-      String_IndexOf |        8 |   5.053 ns |
+      String_IndexOf |        8 |   4.400 ns |
-      String_IndexOf |       15 |   8.817 ns |
+      String_IndexOf |       15 |   4.577 ns |
-      String_IndexOf |       16 |   8.223 ns |
+      String_IndexOf |       16 |   4.697 ns |
-      String_IndexOf |       31 |   9.292 ns |
+      String_IndexOf |       31 |   5.226 ns |
-      String_IndexOf |       32 |   9.276 ns |
+      String_IndexOf |       32 |   5.224 ns |
-      String_IndexOf |       63 |  11.307 ns |
+      String_IndexOf |       63 |   6.991 ns |
-      String_IndexOf |       64 |  11.196 ns |
+      String_IndexOf |       64 |   6.990 ns |
-      String_IndexOf |      127 |  14.386 ns |
+      String_IndexOf |      127 |   9.050 ns |
-      String_IndexOf |      128 |  14.643 ns |
+      String_IndexOf |      128 |   9.228 ns |
-      String_IndexOf |      255 |  21.395 ns |
+      String_IndexOf |      255 |  13.919 ns |
-      String_IndexOf |      256 |  21.452 ns |
+      String_IndexOf |      256 |  13.768 ns |
-      String_IndexOf |     1023 |  68.075 ns |
+      String_IndexOf |     1023 |  49.797 ns |
-      String_IndexOf |     1024 |  68.530 ns |
+      String_IndexOf |     1024 |  49.741 ns |

Will follow up after with the other methods as they depend on the helper methods at the bottom of this one.

/cc @CarolEidt @fiigii @tannergooding @jkotas

Resolves #22762

[benaadams]

Some kind of COM issue?

Windows_NT x64 Checked Build and Test (Jit - TieredCompilation=0 EnableIncompleteISAClass=1 FeatureSIMD=0)

MESSAGE:
cmdLine:D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Interop\\COM\\NativeClients\\Primitives\\Primitives.cmd Timed Out

Return code: -100
Raw output file: D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Reports\\Interop.COM\NativeClients\\Primitives\\Primitives.output.txt
Raw output:
BEGIN EXECUTION
Copying 'D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Tests\\Core_Root\\CoreShim.dll'...
 1 file(s) copied.
 \"D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Tests\\Core_Root\\corerun.exe\" Primitives.exe 
Searching for exe to launch in D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Interop\\COM\\NativeClients\\Primitives...
Launching 'D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Interop\\COM\\NativeClients\\Primitives\\COMClientPrimitives.exe'...

cmdLine:D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Interop\\COM\\NativeClients\\Primitives\\Primitives.cmd Timed Out
Test Harness Exitcode is : -100

To run the test:
> set CORE_ROOT=D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Tests\\Core_Root
> D:\\j\\workspace\\x64_checked_w---8a49c5b2\\bin\\tests\\Windows_NT.x64.Checked\\Interop\\COM\\NativeClients\\Primitives\\Primitives.cmd

Expected: True
Actual: False
+++++++++++++++++++
STACK TRACE:
at Interop_COM._NativeClients_Primitives_Primitives_._NativeClients_Primitives_Primitives_cmd() in D:\j\workspace\x64_checked_w---8a49c5b2\bin\tests\Windows_NT.x64.Checked\TestWrappers\Interop.COM\Interop.COM.XUnitWrapper.cs:line 179

[benaadams]

Rebased for merge conflict

[jkotas]

@tannergooding @GrabYourPitchforks Could you please review this?

[benaadams]

Broke something 😢

[benaadams]

Still an issue on the alignment crossing pages; not seeing it though. Still looking.

@GrabYourPitchforks you'll be happy to know your test System.Tests.StringTests.Ctor_CharPtr_DoesNotAccessInvalidPage is picking it up

[GrabYourPitchforks]

Still looking, but I believe the error is in this line in SpanHelpers.IndexOf:

if ((((nint)Unsafe.AsPointer(ref searchSpace) + (nint)offset) & (nint)(Vector256<ushort>.Count - 1)) != 0)

This essentially treats searchSpace as a byte* (not a char*), adds offset bytes (not chars) to the pointer, then sees if the whole thing is aligned to a 16-byte boundary. This means it's properly SSE2-aligned, but not AVX2-aligned. So a 256-bit read on data that's only guaranteed 128-bit aligned can indeed spill over to the next page and AV.

I believe this line should instead read:

if (((nint)Unsafe.AsPointer(ref Unsafe.Add(ref searchSpace, (IntPtr)offset)) & (nint)(Vector256<byte>.Count - 1)) != 0)

That performs char* (not a byte*) pointer arithmetic on the searchSpace reference, then checks for a 32-byte boundary, not a 16-byte boundary. The data should then be properly AVX2-aligned.

[benaadams]

Still looking, but I believe the error is in this line ...

Ah-ah! Could also cast the return of AsPointer to (char*) prior to the add?

[benaadams]

Btw, nice test :)

[GrabYourPitchforks]

Thanks - glad the test proved fruitful! :)

[benaadams]

Anyone fancy merging? Then I can update the other PRs that depend on the methods in this PR

[tannergooding]

Thanks for the work @benaadams, and sorry for the delay in getting this merged.