Vector128<byte>:get_Zero() doesn't inline (or intrinsicify) at crossgen

[benaadams]

Nor does Vector128<byte>.Count() or Vector128:AsByte(Vector128`1):Vector128`1

ASCIIUtility.WidenAsciiToUtf16_Sse2 calls Vector128<byte>.Zero

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

Lines 1633 to 1637 in fd181c0

	// Then perform an unaligned write of the first part of the input buffer.
	Vector128<byte> zeroVector = Vector128<byte>.Zero;
	utf16FirstHalfVector = Sse2.UnpackLow(asciiVector, zeroVector);

Which ends up reserving stack, making call and reading the stack back to zero a xmm register:

G_M55642_IG05:
       lea      rcx, [rsp+20H]
       call     [Vector128`1:get_Zero():Vector128`1]
       movaps   xmm0, xmmword ptr [rsp+20H]
       movaps   xmm1, xmm6
       punpcklbw xmm1, xmm0
       movdqu   xmmword ptr [rsi], xmm1
       mov      rax, rsi
       shr      rax, 1
       and      rax, 7
       mov      edx, 8
       sub      rdx, rax
       mov      rax, rdx
       sub      rbx, 16

Which is quite inefficient

/cc @tannergooding @GrabYourPitchforks

category:cq
theme:intrinsics
skill-level:expert
cost:medium

[benaadams]

Switching it for default produces

G_M55642_IG05:
       xorps    xmm1, xmm1
       movaps   xmm2, xmm0
       punpcklbw xmm2, xmm1
       movdqu   xmmword ptr [rdx], xmm2
       mov      r9, rdx
       shr      r9, 1
       and      r9, 7
       mov      eax, 8
       sub      rax, r9
       sub      r8, 16

[GrabYourPitchforks]

Seems to be getting optimized correctly under Tier 1, perhaps a crossgen-specific issue?

[benaadams]

Seems to be getting optimized correctly under Tier 1, perhaps a crossgen-specific issue?

Yes

[benaadams]

Nor does Vector128<byte>.Count() or Vector128:AsByte(Vector128`1):Vector128`1

G_M37173_IG09:
       call     [Vector128`1:get_Count():int]
       movsxd   rax, eax
       cmp      rax, rbx
       jg       G_M37173_IG12
       xor      rbp, rbp
       call     [Vector128`1:get_Count():int]
       movsxd   rcx, eax
       sub      rbx, rcx
       test     rbx, rbx
       je       SHORT G_M37173_IG11
						;; bbWeight=0.50 PerfScore 4.75
G_M37173_IG10:
       movups   xmm6, xmmword ptr [rsi+rbp]
       movups   xmm7, xmmword ptr [rdi+rbp]
       lea      rcx, [rsp+60H]
       movaps   xmmword ptr [rsp+20H], xmm6
       lea      rdx, bword ptr [rsp+20H]
       call     [Vector128:AsByte(Vector128`1):Vector128`1]
       lea      rcx, [rsp+50H]
       movaps   xmmword ptr [rsp+20H], xmm7
       lea      rdx, bword ptr [rsp+20H]
       call     [Vector128:AsByte(Vector128`1):Vector128`1]
       movaps   xmm0, xmmword ptr [rsp+60H]

[jkotas]

You can mark these as [NonVersionable]. It will allow inlining in version-resilient R2R, but we have to guarantee that the methods are set in stone - that is safe guarantee to make for these specific ones.

[jkotas]

Ignore my comment. [NonVersionable] is not going to help for these.

[benaadams]

Its in System.Private.CoreLib so that shouldn't be the issue?

[BruceForstall]

@tannergooding @CarolEidt What is the expected behavior here for intrinsics and crossgen?

[tannergooding]

This is currently "by design" as per https://github.com/dotnet/runtime/blob/master/src/coreclr/src/zap/zapinfo.cpp#L2143-L2153. which was originally added in dotnet/coreclr#25365.

Basically, we don't currently treat any of the HWIntrinsic "helper methods" as crossgenable with the reasoning that the code often changes based on what ISA is supported.
That being said, all code paths are equivalent, so we should be able to target the "current baseline" and rely on rejit to produce the better code, like we are doing elsewhere.

@davidwrighton has also been working in the area so he may be able to comment if there would be any problem with doing so.

[benaadams]

Switching
Vector128<byte> zeroVector = Vector128<byte>.Zero;
to
Vector128<byte> zeroVector = default;
Crossgens it to xorps xmm1, xmm1 so should the C# change?

Also Vector128<byte>.Count() is always 16 regardless, should the C# change there (e.g. using in a loop conditional)