Use AggressiveOptimization for intrinsics-based SpanHelpers

[benaadams]

They are very commonly used as the heavy lifting functions of many other methods and their performance is heavily dependant on their inlines working; and code size heavily dependant on branch elimination based on supported architecture.

From: #22127 (comment)

/cc @AndyAyersMS @jkotas @fiigii @kouvel @tannergooding

[benaadams]

I've only added it to methods that actually use the intrinsics directly, not sure if should be other methods also?

[jkotas]

Buffer.Memmove is in the same category.

[fiigii]

Buffer.Memmove is in the same category.

Yes, we may need to make this change for all the CoreLib methods that use SIMD intrinsic.

not sure if should be other methods also?

Adding [AggressiveOptimization] for methods that directly have if(Isa.IsSupported) or if(Vector.IsHardwareAccelerated) should be sufficient.

[benaadams]

Buffer.Memmove is in the same category.

I was looking at that earlier; it gets crossgened as it isn't doing anything non-AoT. If I add a Avx2.IsSupported check to bump it out of crossgen when it gets Jitted it still uses Sse xmm rather than larger registers even though it is operating on 64 byte blocks

G_M6891_IG11:
       C5FA6F02             vmovdqu  xmm0, qword ptr [rdx]
       C5FA7F01             vmovdqu  qword ptr [rcx], xmm0
       C5FA6F4210           vmovdqu  xmm0, qword ptr [rdx+16]
       C5FA7F4110           vmovdqu  qword ptr [rcx+16], xmm0
       C5FA6F4220           vmovdqu  xmm0, qword ptr [rdx+32]
       C5FA7F4120           vmovdqu  qword ptr [rcx+32], xmm0
       C5FA6F4230           vmovdqu  xmm0, qword ptr [rdx+48]
       C5FA7F4130           vmovdqu  qword ptr [rcx+48], xmm0
       4883C140             add      rcx, 64
       4883C240             add      rdx, 64

Don't know if that's a Jit change to use larger registers, or if should add some Avx2 intrinsics here?

Its a fairly well annotated method as to what it wants to happen where

[jkotas]

Yes, we may need to make this change for all the CoreLib methods that use SIMD intrinsic.

I do not think this needs to be made for all methods. It should be made only for the methods that are very likely to be hot if they get used in the workload. MemCpy is definitely in this category. It is less clear for some of the methods in this PR - like LastIndexOfAny.

[jkotas]

I was looking at that earlier; it gets crossgened

I have not realized that.

[benaadams]

Don't know if that's a Jit change to use larger registers

e.g. change copyblk as its using standard structs to do the copies

[StructLayout(LayoutKind.Sequential, Size = 16)]
private struct Block16 { }

[StructLayout(LayoutKind.Sequential, Size = 64)]
private struct Block64 { }

[benaadams]

Jit change would have the advantage of being ready at crossgen as xmm and then when the Tier1 kicked in it would move to ymm?

[benaadams]

Removed from LastIndexOfAny

[stephentoub]

Removed from LastIndexOfAny

Seems like a bunch of the others are potentially problematic, too. For example, I chose a random method that uses IndexOf<T>(ReadOnlySpan<T>, T): Version.ParseVersion goes from 1137 bytes before to 2451 after. And Guid.TryParseVersionX goes from 1653 to 3816.

[jkotas]

Version.ParseVersion goes from 1137 bytes before to 2451 after.

Why would this change make Version.ParseVersion code to change? AggresiveOptimization should not affect inlining, I would think.

[stephentoub]

Why would this change make Version.ParseVersion code to change? AggresiveOptimization should not affect inlining, I would think.

Ugh, sorry, I completely misread the Aggressive* and thought it was AggressiveInlining not AggressiveOptimization. Ignore me.

[benaadams]

coreclr-ci failure looks like infra

Test pri0 Linux_rhel6 x64 checked Job
Agent: Hosted Ubuntu 1604 16

##[section]Starting: Download secrets: ToolsKV
==============================================================================
Task         : Azure Key Vault
Description  : Download Azure Key Vault Secrets
Version      : 1.0.27
Author       : Microsoft Corporation
Help         : [More Information](https://go.microsoft.com/fwlink/?linkid=848891)
==============================================================================
SubscriptionId: a4fc5514-21a9-4296-bfaf-5c7ee7fa35d1.
Key vault name: ToolsKV.
Downloading secret value for: BotAccount-dotnet-github-anon-kaonashi-bot-helix-token.
##[error]
-kaonashi-bot-helix-token: "Request timeout: /***/oauth2/token/"
##[section]Finishing: Download secrets: ToolsKV

[benaadams]

Is there anyway to retrigger a sub-job of coreclr-ci?

[stephentoub]

Is there anyway to retrigger a sub-job of coreclr-ci?

Yes, though it's not obvious and can't currently be done here via commands to a bot (I also don't know if you need special permissions). Basically:

• Click the Details link, which will take you to the Checks page.
• Click the tiny "View more details on Azure Pipelines" link at the bottom
• Click on the leg you want to re-run. Then click the "..." on the upper-right, and select Re-run from the menu.

[stephentoub]

cc: @safern

[benaadams]

I also don't know if you need special permissions

Yeah, looks like it, I only have "Download the logs"

I'll have to live vicariously though people that work at MS :)

[safern]

(I also don't know if you need special permissions)

You need to be logged in and have permissions to do that. We've given that feedback and also asked for the feature to be able to retry it from a comment (rather than queue the whole build again). Also it seems like the Re-run button from the github UI is broken. All those issues are being raised and prioritized so that we can improve the experience. cc: @chcosta for your meeting tomorrow.