Tagging subscribers to this area:@JulieLeeMSFT,@jakobbotsch
See info inarea-owners.md if you want to be subscribed.

publicstaticVector128<byte>MultiplyB128(Vector128<byte>v1,Vector128<byte>v2)=>v1*v2;

G_M65351_IG01:  ;; offset=0000Hvzeroupper;; size=3 bbWeight=1 PerfScore 1.00G_M65351_IG02:  ;; offset=0003H       vpmovzxbwymm0, qword ptr[rdx]       vpmovzxbwymm1, qword ptr[r8]       vpmullwymm0,ymm0,ymm1vpandymm0,ymm0, ymmword ptr[reloc @RWD00]       vpackuswbymm0,ymm0,ymm0vpermqymm0,ymm0,-40vmovups  xmmword ptr[rcx],xmm0movrax,rcx;; size=39 bbWeight=1 PerfScore 21.25G_M65351_IG03:  ;; offset=002AHvzeroupperret;; size=4 bbWeight=1 PerfScore 2.00RWD00  dq00FF00FF00FF00FFh,00FF00FF00FF00FFh,00FF00FF00FF00FFh,00FF00FF00FF00FFh

I have updated the PR to fix the issue with AVX512.
Source code got longer to address SIMD16 handling with and without AVX512BW_VL support.

@tannergooding

@tannergooding, please review this community contribution and consider if this can be done in C# instead of in JIT.

Could this code instead be implemented in C# in (for example) the Vector128::operator* fallback implementation, using all the same hardware intrinsics, ISA checking, etc.? Or is there some reason why that either can't or shouldn't be done?

It "could" be, but there are certain pessimizations that I believe make doing that the wrong choice for these APIs. Most notably because these APIs are considered "perf critical" and one of the primary ways users interact with the SIMD types.

One of the pessimizations is that the IR produced by inlining still differs and is "sub-optimal" in comparison. It is often inserting additional copies, locals, temporaries, or other handling at the "boundary" just before/after the inlined code. A lot of this has been improved with the introduction of forward sub, but there are still notable edge cases. Likewise, it is still dependent on the inliner and while we've made some improvements to that (particularly in adjusting the budget when many intrinsic calls exist), there are still plenty of cases (especially in more complex methods) where the budget is quickly eaten by APIs like this being in managed and it will simply give up. That is often less problematic for other APIs, but represents a particular concern when you're considering APIs that are exposing primitive functionality used to write all your other algorithms.

We ultimately are generating roughly 6 instructions of very specialized code for what is a baseline arithmetic operation on the SIMD types. If we were designing IL "today", we would potentially have SIMD type support built into the IL and so this would be the same handling we'd be doing forCEE_MUL in that case. Because its a baseline arithmetic operation, we want such cases to participate in VN, CSE, potentially in constant folding, to be properly costed, etc.

Notably, it would probably be better for this logic to be in lowering or morph instead and for us to just track it asVector128_op_Multiply in the front end. That's how we handle other primitive operations that tend to be slightly more expensive on some platforms as well (such as conversions, checked arithmetic, long decomposition, etc). But, as is, its consistent with the existing handling for other similar scenarios..

Longer term I would definitely like to see the issues called out above addressed, even if that's done via special casing these APIs in the inliner/etc instead. Once we're confident that's the case, then we should look at incrementally moving all similar functionality into managed instead. We might still end up with some bits staying as handled by the JIT (such as for truly simple operations where equivalent handling exists forint,long,float, ordouble), but it would allow all ofsimdashwintrinsic to be removed (they are really user defined types that are getting special optimizations today and are not ABI primitives) and many of the more complex helpers (likeDot orSum) to have their complexity removed from the JIT.

Since it is past .NET 8 code complete, setting this to .NET 9.

@BruceForstall I have merged the branch and fixed the break with current sources.
I will adress the other concerns (more comments with examples and specific variables for new nodes) as soon as tests are completed.

@DeepakRajendrakumaran@anthonycanino@tannergooding @dotnet/avx512-contrib Any additional comments/suggestions on this code?

Sorry for the late reply. Changes look good at first glance. Couldn't find any similar implementation in native compilers to compare disasm though

@BruceForstall as per your request:

1. I have re-merged with latest main and addressed other concerns.
2. More comments have been added to explain what each branch wants to do, and samples of the equivalent C# code for relevant statements have been provided. I did not provide an example of the generated ASM because it could be too verbose in some cases. Let me know if you prefer I add it anyway in sources or as a comment to this PR.
3. All new nodes now use new variables, named to explicitly indicate the contents.

Test failures seem unrelated to the PR (Build Analysis failurelogs report 'No space left on device').
Is it possible to re-run failed checks?

/azp run runtime-coreclr superpmi-diffs

/azp run runtime-coreclr jitstress-isas-avx512, runtime-coreclr jitstress-isas-x86

Failing checks are related to#93163.

@BruceForstall thanks to you for reviewing and support!