Check Avx512BW.IsSupported in BitArray.CopyTo()

[tfenise]

runtime/src/libraries/System.Collections/src/System/Collections/BitArray.cs

Line 855 in 7e297e6

Vector512<byte> shuffled = Avx512BW.Shuffle(scalar.AsByte(), shuffleMask);

runtime/src/libraries/System.Collections/src/System/Collections/BitArray.cs

Line 860 in 7e297e6

Vector512<byte> normalized = Avx512BW.Min(extracted, ones);

It's using Avx512BW, so it should check Avx512BW.IsSupported first. Checking Avx512F.IsSupported is not enough, as there are (old) CPUs supporting Avx512F but not Avx512BW according to https://en.wikipedia.org/w/index.php?title=AVX-512&oldid=1281313848#CPUs_with_AVX-512.

[dotnet-policy-service]

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

[tannergooding]

Since you're touching this anyways, would you want to update this to use the xplat APIs instead?

That is, swap these out:

• Avx512BW.IsSupported -> Vector512.IsHardwareAccelerated
• Avx512BW.Shuffle(x, y) -> Vector512.Shuffle(x, y)
• Avx512F.And(x, y) -> x & y
• Avx512BW.Min(x, y) -> Vector512.Min(x, y)
• Avx512F.Store(x, y) -> y.Store(x)

Similar changes could also be made to the Avx2 path (using Vector256) and so on.

---

This change to Avx512BW is notably "more correct" than Avx512F, but notably it's not technically broken today since we require AVX512F+BW+CD+DQ+VL to be provided for any AVX512 support to exist. It's still goodness to fix, but moving away from the platform specific APIs altogether would be even better.

[tfenise]

About Vector512.Shuffle(x, y)

runtime/src/libraries/System.Collections/src/System/Collections/BitArray.cs

Lines 928 to 930 in 1a51788

	// Same logic as SSSE3 path, except we do not have Shuffle instruction.
	// (TableVectorLookup could be an alternative - dotnet/runtime#1277)
	// Instead we use chained ZIP1/2 instructions:

It seems that Arm64 might not have the proper instruction to support VectorXXX.Shuffle<byte>(x, y). Could it happen that VectorXXX.IsHardwareAccelerated is true but VectorXXX.Shuffle<byte>(x, y) has poor performance on some platforms?

[tannergooding]

It seems that Arm64 might not have the proper instruction to support VectorXXX.Shuffle(x, y). C

That comment is very outdated, VectorXXX.Shuffle covers the usage and does the correct thing.

Could it happen that VectorXXX.IsHardwareAccelerated is true but VectorXXX.Shuffle(x, y) has poor performance on some platforms?

Not for the way we're using it here. Theoretically it could happen if you were on a machine from 2005 and didn't have SSSE3 but hit the Vector128.IsHardwareAccelerated code path. This isn't really a concern for anyone using the current versions of .NET and could be adjusted later if it was a concern.

[saucecontrol]

Today, Vector256.IsHardwareAccelerated strictly implies AVX2 support, and Vector512.IsHardwareAccelerated strictly implies AVX-512F+CD+DQ+BW+VL. Arm64 support for larger vectors comes in the form of Scalable Vector Extensions (SVE), which does not allow acceleration of the fixed-width vector types.

The Vector128 code in BitArray is outdated and could also use the xplat intrinsics.

[tfenise]

Actually, after the runtime has proper AVX512 masking support #87097, the AVX512 path should probably use kmovq k, m64 and use that mask. This gives a reason to promote #87097. Will xplat Vector512 offer the same capacity?

[saucecontrol]

Shuffle attempts to normalize behavior across platforms, including zeroing out of range indices and allowing full vector (cross-lane) shuffle/permute. If you don't need that behavior, you can use the new Vector512.ShuffleNative, which will emit the expected vpshufb.

vpermb is actually AVX512_VBMI, not just AVX-512F+CD+DQ+BW+VL

This is not a problem. The baseline ISA set is required for any acceleration of Vector512, but JIT can opportunistically use any ISA supported by the hardware. One of the benefits of the xplat vector APIs is that JIT can optimize or polyfill using whatever instructions are available.

[stephentoub]

Since you're touching this anyways, would you want to update this to use the xplat APIs instead?

@tannergooding , should this PR be merged or closed in the interim?

[tannergooding]

Lets take it, as it is technically correct and will ensure valid behavior in the case other runtimes use the same libraries and have their own distinct support. (There's no bug on RyuJIT today due to how we've defined things to work for ourselves)

[tannergooding]

Logged #116079 to track the move to xplat APIs

[tannergooding]

/ba-g unrelated build failures, simply updated an Isa.IsSupported check to be more correct