GetIndexOfFirstNonAsciiByte_Vector path in Ascii.Utility.cs is never exercised for AdvSimd/SSE4.1

[neon-sunset]

Description

It appears that GetIndexOfFirstNonAsciiByte in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Text/Ascii.Utility.cs#L104 does not exercise the GetIndexOfFirstNonAsciiByte_Vector path despite it having explicit support for AdvSimd and SSE4.1.

Is this intended? Should SSE4.1 and AdvSimd targets be allowed to take _Vector instead?

Regression?

No

Data

Preliminary benchmark shows that on osx-arm64 apple-m1 the time to traverse 1_000_000 bytes is 28.71us with current _Intrinsified path and is improved by ~40% to 20.12us if we relax the check and allow calling GetIndexOfFirstNonAsciiByte_Vector for Vector128.IsHardwareAccelerated.

[EgorBo]

Looks like a regression (or leftover) from #88532 cc @anthonycanino

[ghost]

Tagging subscribers to this area: @dotnet/area-system-text-encoding
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

It appears that GetIndexOfFirstNonAsciiByte in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Text/Ascii.Utility.cs#L104 does not exercise the GetIndexOfFirstNonAsciiByte_Vector path despite it having explicit support for AdvSimd and SSE4.1.

Is this intended? Should SSE4.1 and AdvSimd targets be allowed to take _Vector instead?

Regression?

No.

Data

Preliminary benchmark shows that on osx-arm64 apple-m1 the time to traverse 1_000_000 bytes is 28.71us with current _Intrinsified path and is improved by ~40% to 20.12us if we relax the check and allow calling GetIndexOfFirstNonAsciiByte_Vector for Vector128.IsHardwareAccelerated.

Author:	neon-sunset
Assignees:	-
Labels:	area-System.Text.Encoding, tenet-performance, untriaged
Milestone:	-

[stephentoub]

Regression? Yes, vs .NET 7

@neon-sunset, @tannergooding pointed out to me offline it was using the _Intrinsified path in .NET 7:

runtime/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs

Lines 81 to 91 in a6dbb80

	public static unsafe nuint GetIndexOfFirstNonAsciiByte(byte* pBuffer, nuint bufferLength)
	{
	// If SSE2 is supported, use those specific intrinsics instead of the generic vectorized
	// code below. This has two benefits: (a) we can take advantage of specific instructions like
	// pmovmskb which we know are optimized, and (b) we can avoid downclocking the processor while
	// this method is running.
	return (Sse2.IsSupported || (AdvSimd.Arm64.IsSupported && BitConverter.IsLittleEndian))
	? GetIndexOfFirstNonAsciiByte_Intrinsified(pBuffer, bufferLength)
	: GetIndexOfFirstNonAsciiByte_Default(pBuffer, bufferLength);
	}

Did you measure it to be a regression from .NET 7, and if so, can you share the benchmark and your resulting numbers?

Or are you just saying it's not as good in .NET 8 as it could be but it's not a regression?

EDIT: Tanner just commented on the PR with similar feedback:
#90527 (comment)

[neon-sunset]

Or are you just saying it's not as good in .NET 8 as it could be but it's not a regression?

Yes, it appears to not be a regression because I mistakenly assumed the _Vector path was present in .NET 7. I have updated the description.