From 15f6a0b0e770aa05e7217780b0812f7060667f99 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 25 Jul 2022 14:08:47 +0200 Subject: [PATCH 1/9] port BitArray.CopyTo to cross platform APIs --- .../src/System/Collections/BitArray.cs | 60 +++++++++---------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 1c6240f4311319..7e7450a4d6bbb5 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -816,29 +816,28 @@ public unsafe void CopyTo(Array array, int index) Vector128 lowerShuffleMask_CopyToBoolArray = Vector128.Create(0, 0x01010101_01010101).AsByte(); Vector128 upperShuffleMask_CopyToBoolArray = Vector128.Create(0x02020202_02020202, 0x03030303_03030303).AsByte(); - if (Avx2.IsSupported) + if (Vector256.IsHardwareAccelerated) { Vector256 shuffleMask = Vector256.Create(lowerShuffleMask_CopyToBoolArray, upperShuffleMask_CopyToBoolArray); Vector256 bitMask = Vector256.Create(0x80402010_08040201).AsByte(); Vector256 ones = Vector256.Create((byte)1); - fixed (bool* destination = &boolArray[index]) + ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); + + for (; (i + Vector256ByteCount) <= (uint)m_length; i += Vector256ByteCount) { - for (; (i + Vector256ByteCount) <= (uint)m_length; i += Vector256ByteCount) - { - int bits = m_array[i / (uint)BitsPerInt32]; - Vector256 scalar = Vector256.Create(bits); - Vector256 shuffled = Avx2.Shuffle(scalar.AsByte(), shuffleMask); - Vector256 extracted = Avx2.And(shuffled, bitMask); - - // The extracted bits can be anywhere between 0 and 255, so we normalise the value to either 0 or 1 - // to ensure compatibility with "C# bool" (0 for false, 1 for true, rest undefined) - Vector256 normalized = Avx2.Min(extracted, ones); - Avx.Store((byte*)destination + i, normalized); - } + int bits = m_array[i / (uint)BitsPerInt32]; + Vector256 scalar = Vector256.Create(bits); + Vector256 shuffled = Vector256.Shuffle(scalar.AsByte(), shuffleMask); + Vector256 extracted = shuffled & bitMask; + + // The extracted bits can be anywhere between 0 and 255, so we normalise the value to either 0 or 1 + // to ensure compatibility with "C# bool" (0 for false, 1 for true, rest undefined) + Vector256 normalized = Vector256.Min(extracted, ones); + normalized.StoreUnsafe(ref destination, i); } } - else if (Ssse3.IsSupported) + else if (Vector128.IsHardwareAccelerated) { Vector128 lowerShuffleMask = lowerShuffleMask_CopyToBoolArray; Vector128 upperShuffleMask = upperShuffleMask_CopyToBoolArray; @@ -847,23 +846,22 @@ public unsafe void CopyTo(Array array, int index) Vector128.Create(0x80402010_08040201).AsByte() : Vector128.Create(0x01020408_10204080).AsByte(); - fixed (bool* destination = &boolArray[index]) - { - for (; (i + Vector128ByteCount * 2u) <= (uint)m_length; i += Vector128ByteCount * 2u) - { - int bits = m_array[i / (uint)BitsPerInt32]; - Vector128 scalar = Vector128.CreateScalarUnsafe(bits); + ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); - Vector128 shuffledLower = Ssse3.Shuffle(scalar.AsByte(), lowerShuffleMask); - Vector128 extractedLower = Sse2.And(shuffledLower, bitMask128); - Vector128 normalizedLower = Sse2.Min(extractedLower, ones); - Sse2.Store((byte*)destination + i, normalizedLower); - - Vector128 shuffledHigher = Ssse3.Shuffle(scalar.AsByte(), upperShuffleMask); - Vector128 extractedHigher = Sse2.And(shuffledHigher, bitMask128); - Vector128 normalizedHigher = Sse2.Min(extractedHigher, ones); - Sse2.Store((byte*)destination + i + Vector128.Count, normalizedHigher); - } + for (; (i + Vector128ByteCount * 2u) <= (uint)m_length; i += Vector128ByteCount * 2u) + { + int bits = m_array[i / (uint)BitsPerInt32]; + Vector128 scalar = Vector128.CreateScalarUnsafe(bits); + + Vector128 shuffledLower = Vector128.Shuffle(scalar.AsByte(), lowerShuffleMask); + Vector128 extractedLower = shuffledLower & bitMask128; + Vector128 normalizedLower = Vector128.Min(extractedLower, ones); + normalizedLower.StoreUnsafe(ref destination, i); + + Vector128 shuffledHigher = Vector128.Shuffle(scalar.AsByte(), upperShuffleMask); + Vector128 extractedHigher = shuffledHigher & bitMask128; + Vector128 normalizedHigher = Vector128.Min(extractedHigher, ones); + normalizedHigher.StoreUnsafe(ref destination, (nuint)(i + Vector128.Count)); } } else if (AdvSimd.Arm64.IsSupported) From c64ed21b9d3e3fda1e2bd7cd324925e51deb32dc Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 25 Jul 2022 14:26:05 +0200 Subject: [PATCH 2/9] remove ARM code path --- .../src/System/Collections/BitArray.cs | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 7e7450a4d6bbb5..0af5b13f98954a 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -864,49 +864,6 @@ public unsafe void CopyTo(Array array, int index) normalizedHigher.StoreUnsafe(ref destination, (nuint)(i + Vector128.Count)); } } - else if (AdvSimd.Arm64.IsSupported) - { - Vector128 ones = Vector128.Create((byte)1); - Vector128 bitMask128 = BitConverter.IsLittleEndian ? - Vector128.Create(0x80402010_08040201).AsByte() : - Vector128.Create(0x01020408_10204080).AsByte(); - - fixed (bool* destination = &boolArray[index]) - { - for (; (i + Vector128ByteCount * 2u) <= (uint)m_length; i += Vector128ByteCount * 2u) - { - int bits = m_array[i / (uint)BitsPerInt32]; - // 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: - // (A0 is the byte containing LSB, A3 is the byte containing MSB) - // bits (on Big endian) - A3 A2 A1 A0 - // bits (Little endian) / Byte reversal - A0 A1 A2 A3 - // v1 = Vector128.Create - A0 A1 A2 A3 A0 A1 A2 A3 A0 A1 A2 A3 A0 A1 A2 A3 - // v2 = ZipLow(v1, v1) - A0 A0 A1 A1 A2 A2 A3 A3 A0 A0 A1 A1 A2 A2 A3 A3 - // v3 = ZipLow(v2, v2) - A0 A0 A0 A0 A1 A1 A1 A1 A2 A2 A2 A2 A3 A3 A3 A3 - // shuffledLower = ZipLow(v3, v3) - A0 A0 A0 A0 A0 A0 A0 A0 A1 A1 A1 A1 A1 A1 A1 A1 - // shuffledHigher = ZipHigh(v3, v3) - A2 A2 A2 A2 A2 A2 A2 A2 A3 A3 A3 A3 A3 A3 A3 A3 - if (!BitConverter.IsLittleEndian) - { - bits = BinaryPrimitives.ReverseEndianness(bits); - } - Vector128 vector = Vector128.Create(bits).AsByte(); - vector = AdvSimd.Arm64.ZipLow(vector, vector); - vector = AdvSimd.Arm64.ZipLow(vector, vector); - - Vector128 shuffledLower = AdvSimd.Arm64.ZipLow(vector, vector); - Vector128 extractedLower = AdvSimd.And(shuffledLower, bitMask128); - Vector128 normalizedLower = AdvSimd.Min(extractedLower, ones); - - Vector128 shuffledHigher = AdvSimd.Arm64.ZipHigh(vector, vector); - Vector128 extractedHigher = AdvSimd.And(shuffledHigher, bitMask128); - Vector128 normalizedHigher = AdvSimd.Min(extractedHigher, ones); - - AdvSimd.Arm64.StorePair((byte*)destination + i, normalizedLower, normalizedHigher); - } - } - } LessThan32: for (; i < (uint)m_length; i++) From 1a345d42b80e7c13bea0d0d6d783db95ab3ca3d4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 25 Jul 2022 15:05:47 +0200 Subject: [PATCH 3/9] remove unnecessary usings --- .../System.Collections/src/System/Collections/BitArray.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 0af5b13f98954a..7e41a6bb778744 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -6,8 +6,6 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; -using System.Runtime.Intrinsics.X86; -using System.Runtime.Intrinsics.Arm; namespace System.Collections { From 918baf12c96cb5d2fd1a3963d5d969ec8796c740 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 26 Jul 2022 14:42:53 +0200 Subject: [PATCH 4/9] restore Avx2 code path as Vector256.Shuffle does not produce optimal codegen as of now --- .../src/System/Collections/BitArray.cs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 7e41a6bb778744..3e54f4d07efd67 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -6,6 +6,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.X86; namespace System.Collections { @@ -814,25 +815,26 @@ public unsafe void CopyTo(Array array, int index) Vector128 lowerShuffleMask_CopyToBoolArray = Vector128.Create(0, 0x01010101_01010101).AsByte(); Vector128 upperShuffleMask_CopyToBoolArray = Vector128.Create(0x02020202_02020202, 0x03030303_03030303).AsByte(); - if (Vector256.IsHardwareAccelerated) + if (Avx2.IsSupported) { Vector256 shuffleMask = Vector256.Create(lowerShuffleMask_CopyToBoolArray, upperShuffleMask_CopyToBoolArray); Vector256 bitMask = Vector256.Create(0x80402010_08040201).AsByte(); Vector256 ones = Vector256.Create((byte)1); - ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); - - for (; (i + Vector256ByteCount) <= (uint)m_length; i += Vector256ByteCount) + fixed (bool* destination = &boolArray[index]) { - int bits = m_array[i / (uint)BitsPerInt32]; - Vector256 scalar = Vector256.Create(bits); - Vector256 shuffled = Vector256.Shuffle(scalar.AsByte(), shuffleMask); - Vector256 extracted = shuffled & bitMask; - - // The extracted bits can be anywhere between 0 and 255, so we normalise the value to either 0 or 1 - // to ensure compatibility with "C# bool" (0 for false, 1 for true, rest undefined) - Vector256 normalized = Vector256.Min(extracted, ones); - normalized.StoreUnsafe(ref destination, i); + for (; (i + Vector256ByteCount) <= (uint)m_length; i += Vector256ByteCount) + { + int bits = m_array[i / (uint)BitsPerInt32]; + Vector256 scalar = Vector256.Create(bits); + Vector256 shuffled = Avx2.Shuffle(scalar.AsByte(), shuffleMask); + Vector256 extracted = Avx2.And(shuffled, bitMask); + + // The extracted bits can be anywhere between 0 and 255, so we normalise the value to either 0 or 1 + // to ensure compatibility with "C# bool" (0 for false, 1 for true, rest undefined) + Vector256 normalized = Avx2.Min(extracted, ones); + Avx.Store((byte*)destination + i, normalized); + } } } else if (Vector128.IsHardwareAccelerated) From 7965d1553297a7cd2bdf2bb1e39e9f31497dfafd Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 26 Jul 2022 14:52:03 +0200 Subject: [PATCH 5/9] optimize Vector128 codepath to avoid regression --- .../src/System/Collections/BitArray.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 3e54f4d07efd67..06d23c61747961 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -809,15 +809,15 @@ public unsafe void CopyTo(Array array, int index) if (m_length < BitsPerInt32) goto LessThan32; - // The mask used when shuffling a single int into Vector128/256. - // On little endian machines, the lower 8 bits of int belong in the first byte, next lower 8 in the second and so on. - // We place the bytes that contain the bits to its respective byte so that we can mask out only the relevant bits later. - Vector128 lowerShuffleMask_CopyToBoolArray = Vector128.Create(0, 0x01010101_01010101).AsByte(); - Vector128 upperShuffleMask_CopyToBoolArray = Vector128.Create(0x02020202_02020202, 0x03030303_03030303).AsByte(); - if (Avx2.IsSupported) { - Vector256 shuffleMask = Vector256.Create(lowerShuffleMask_CopyToBoolArray, upperShuffleMask_CopyToBoolArray); + // The mask used when shuffling a single int into Vector128/256. + // On little endian machines, the lower 8 bits of int belong in the first byte, next lower 8 in the second and so on. + // We place the bytes that contain the bits to its respective byte so that we can mask out only the relevant bits later. + Vector128 lowerShuffleMask = Vector128.Create(0, 0x01010101_01010101).AsByte(); + Vector128 upperShuffleMask = Vector128.Create(0x02020202_02020202, 0x03030303_03030303).AsByte(); + + Vector256 shuffleMask = Vector256.Create(lowerShuffleMask, upperShuffleMask); Vector256 bitMask = Vector256.Create(0x80402010_08040201).AsByte(); Vector256 ones = Vector256.Create((byte)1); @@ -839,8 +839,6 @@ public unsafe void CopyTo(Array array, int index) } else if (Vector128.IsHardwareAccelerated) { - Vector128 lowerShuffleMask = lowerShuffleMask_CopyToBoolArray; - Vector128 upperShuffleMask = upperShuffleMask_CopyToBoolArray; Vector128 ones = Vector128.Create((byte)1); Vector128 bitMask128 = BitConverter.IsLittleEndian ? Vector128.Create(0x80402010_08040201).AsByte() : @@ -853,12 +851,14 @@ public unsafe void CopyTo(Array array, int index) int bits = m_array[i / (uint)BitsPerInt32]; Vector128 scalar = Vector128.CreateScalarUnsafe(bits); - Vector128 shuffledLower = Vector128.Shuffle(scalar.AsByte(), lowerShuffleMask); + // the shuffle masks used below are the same as masks used by the Avx2 codepath above + // they need to be consts for optimal codegen (see https://github.com/dotnet/runtime/issues/72793) + Vector128 shuffledLower = Vector128.Shuffle(scalar.AsByte(), Vector128.Create((byte)0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1)); Vector128 extractedLower = shuffledLower & bitMask128; Vector128 normalizedLower = Vector128.Min(extractedLower, ones); normalizedLower.StoreUnsafe(ref destination, i); - Vector128 shuffledHigher = Vector128.Shuffle(scalar.AsByte(), upperShuffleMask); + Vector128 shuffledHigher = Vector128.Shuffle(scalar.AsByte(), Vector128.Create((byte)2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3)); Vector128 extractedHigher = shuffledHigher & bitMask128; Vector128 normalizedHigher = Vector128.Min(extractedHigher, ones); normalizedHigher.StoreUnsafe(ref destination, (nuint)(i + Vector128.Count)); From 7d4757c78b12724cb2a7b54945c506ff0fe2dae2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 26 Jul 2022 15:15:36 +0200 Subject: [PATCH 6/9] address code review feedback: use Vector128.Count instead of a private const --- .../src/System/Collections/BitArray.cs | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 06d23c61747961..029ea68b05e2a0 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -117,10 +117,6 @@ public BitArray(byte[] bytes) _version = 0; } - private const uint Vector128ByteCount = 16; - private const uint Vector128IntCount = 4; - private const uint Vector256ByteCount = 32; - private const uint Vector256IntCount = 8; public unsafe BitArray(bool[] values) { ArgumentNullException.ThrowIfNull(values); @@ -143,7 +139,7 @@ public unsafe BitArray(bool[] values) if (Vector256.IsHardwareAccelerated) { - for (; (i + Vector256ByteCount) <= (uint)values.Length; i += Vector256ByteCount) + for (; (i + Vector256.Count) <= (uint)values.Length; i += (uint)Vector256.Count) { Vector256 vector = Vector256.LoadUnsafe(ref value, i); Vector256 isFalse = Vector256.Equals(vector, Vector256.Zero); @@ -154,13 +150,13 @@ public unsafe BitArray(bool[] values) } else if (Vector128.IsHardwareAccelerated) { - for (; (i + Vector128ByteCount * 2u) <= (uint)values.Length; i += Vector128ByteCount * 2u) + for (; (i + Vector128.Count * 2u) <= (uint)values.Length; i += (uint)Vector128.Count * 2u) { Vector128 lowerVector = Vector128.LoadUnsafe(ref value, i); Vector128 lowerIsFalse = Vector128.Equals(lowerVector, Vector128.Zero); uint lowerResult = lowerIsFalse.ExtractMostSignificantBits(); - Vector128 upperVector = Vector128.LoadUnsafe(ref value, i + Vector128ByteCount); + Vector128 upperVector = Vector128.LoadUnsafe(ref value, i + (uint)Vector128.Count); Vector128 upperIsFalse = Vector128.Equals(upperVector, Vector128.Zero); uint upperResult = upperIsFalse.ExtractMostSignificantBits(); @@ -344,7 +340,7 @@ public unsafe BitArray And(BitArray value) if (Vector256.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector256IntCount - 1u); i += Vector256IntCount) + for (; i < (uint)count - (Vector256.Count - 1u); i += (uint)Vector256.Count) { Vector256 result = Vector256.LoadUnsafe(ref left, i) & Vector256.LoadUnsafe(ref right, i); result.StoreUnsafe(ref left, i); @@ -352,7 +348,7 @@ public unsafe BitArray And(BitArray value) } else if (Vector128.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector128IntCount - 1u); i += Vector128IntCount) + for (; i < (uint)count - (Vector128.Count - 1u); i += (uint)Vector128.Count) { Vector128 result = Vector128.LoadUnsafe(ref left, i) & Vector128.LoadUnsafe(ref right, i); result.StoreUnsafe(ref left, i); @@ -410,7 +406,7 @@ public unsafe BitArray Or(BitArray value) if (Vector256.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector256IntCount - 1u); i += Vector256IntCount) + for (; i < (uint)count - (Vector256.Count - 1u); i += (uint)Vector256.Count) { Vector256 result = Vector256.LoadUnsafe(ref left, i) | Vector256.LoadUnsafe(ref right, i); result.StoreUnsafe(ref left, i); @@ -418,7 +414,7 @@ public unsafe BitArray Or(BitArray value) } else if (Vector128.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector128IntCount - 1u); i += Vector128IntCount) + for (; i < (uint)count - (Vector128.Count - 1u); i += (uint)Vector128.Count) { Vector128 result = Vector128.LoadUnsafe(ref left, i) | Vector128.LoadUnsafe(ref right, i); result.StoreUnsafe(ref left, i); @@ -476,7 +472,7 @@ public unsafe BitArray Xor(BitArray value) if (Vector256.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector256IntCount - 1u); i += Vector256IntCount) + for (; i < (uint)count - (Vector256.Count - 1u); i += (uint)Vector256.Count) { Vector256 result = Vector256.LoadUnsafe(ref left, i) ^ Vector256.LoadUnsafe(ref right, i); result.StoreUnsafe(ref left, i); @@ -484,7 +480,7 @@ public unsafe BitArray Xor(BitArray value) } else if (Vector128.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector128IntCount - 1u); i += Vector128IntCount) + for (; i < (uint)count - (Vector128.Count - 1u); i += (uint)Vector128.Count) { Vector128 result = Vector128.LoadUnsafe(ref left, i) ^ Vector128.LoadUnsafe(ref right, i); result.StoreUnsafe(ref left, i); @@ -534,7 +530,7 @@ public unsafe BitArray Not() if (Vector256.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector256IntCount - 1u); i += Vector256IntCount) + for (; i < (uint)count - (Vector256.Count - 1u); i += (uint)Vector256.Count) { Vector256 result = ~Vector256.LoadUnsafe(ref value, i); result.StoreUnsafe(ref value, i); @@ -542,7 +538,7 @@ public unsafe BitArray Not() } else if (Vector128.IsHardwareAccelerated) { - for (; i < (uint)count - (Vector128IntCount - 1u); i += Vector128IntCount) + for (; i < (uint)count - (Vector128.Count - 1u); i += (uint)Vector128.Count) { Vector128 result = ~Vector128.LoadUnsafe(ref value, i); result.StoreUnsafe(ref value, i); @@ -823,7 +819,7 @@ public unsafe void CopyTo(Array array, int index) fixed (bool* destination = &boolArray[index]) { - for (; (i + Vector256ByteCount) <= (uint)m_length; i += Vector256ByteCount) + for (; (i + Vector256.Count) <= (uint)m_length; i += (uint)Vector256.Count) { int bits = m_array[i / (uint)BitsPerInt32]; Vector256 scalar = Vector256.Create(bits); @@ -846,7 +842,7 @@ public unsafe void CopyTo(Array array, int index) ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); - for (; (i + Vector128ByteCount * 2u) <= (uint)m_length; i += Vector128ByteCount * 2u) + for (; (i + Vector128.Count * 2u) <= (uint)m_length; i += (uint)Vector128.Count * 2u) { int bits = m_array[i / (uint)BitsPerInt32]; Vector128 scalar = Vector128.CreateScalarUnsafe(bits); From 2ea70af03c4bd615826a7849e85145f324a971d3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 26 Jul 2022 15:48:08 +0200 Subject: [PATCH 7/9] address code review feedback: access array as local variable rather than field --- .../src/System/Collections/BitArray.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 029ea68b05e2a0..715f3fe2c4890a 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -801,6 +801,8 @@ public unsafe void CopyTo(Array array, int index) } uint i = 0; + int[] thisArray = m_array; + uint thisLength = (uint)m_length; if (m_length < BitsPerInt32) goto LessThan32; @@ -819,9 +821,9 @@ public unsafe void CopyTo(Array array, int index) fixed (bool* destination = &boolArray[index]) { - for (; (i + Vector256.Count) <= (uint)m_length; i += (uint)Vector256.Count) + for (; (i + Vector256.Count) <= thisLength; i += (uint)Vector256.Count) { - int bits = m_array[i / (uint)BitsPerInt32]; + int bits = thisArray[i / (uint)BitsPerInt32]; Vector256 scalar = Vector256.Create(bits); Vector256 shuffled = Avx2.Shuffle(scalar.AsByte(), shuffleMask); Vector256 extracted = Avx2.And(shuffled, bitMask); @@ -842,9 +844,9 @@ public unsafe void CopyTo(Array array, int index) ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); - for (; (i + Vector128.Count * 2u) <= (uint)m_length; i += (uint)Vector128.Count * 2u) + for (; (i + Vector128.Count * 2u) <= thisLength; i += (uint)Vector128.Count * 2u) { - int bits = m_array[i / (uint)BitsPerInt32]; + int bits = thisArray[i / (uint)BitsPerInt32]; Vector128 scalar = Vector128.CreateScalarUnsafe(bits); // the shuffle masks used below are the same as masks used by the Avx2 codepath above @@ -862,10 +864,10 @@ public unsafe void CopyTo(Array array, int index) } LessThan32: - for (; i < (uint)m_length; i++) + for (; i < thisLength; i++) { int elementIndex = Div32Rem((int)i, out int extraBits); - boolArray[(uint)index + i] = ((m_array[elementIndex] >> extraBits) & 0x00000001) != 0; + boolArray[(uint)index + i] = ((thisArray[elementIndex] >> extraBits) & 0x00000001) != 0; } } else From 5aca3255914d4c52516503e7bb97bd1a1667754e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 26 Jul 2022 15:55:53 +0200 Subject: [PATCH 8/9] address code review feedback: optimize loop --- .../System.Collections/src/System/Collections/BitArray.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index 715f3fe2c4890a..ffc472648bd197 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -821,7 +821,7 @@ public unsafe void CopyTo(Array array, int index) fixed (bool* destination = &boolArray[index]) { - for (; (i + Vector256.Count) <= thisLength; i += (uint)Vector256.Count) + for (; i <= thisLength - Vector256.Count; i += (uint)Vector256.Count) { int bits = thisArray[i / (uint)BitsPerInt32]; Vector256 scalar = Vector256.Create(bits); @@ -844,7 +844,7 @@ public unsafe void CopyTo(Array array, int index) ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); - for (; (i + Vector128.Count * 2u) <= thisLength; i += (uint)Vector128.Count * 2u) + for (; i <= thisLength - Vector128.Count * 2u; i += (uint)Vector128.Count * 2u) { int bits = thisArray[i / (uint)BitsPerInt32]; Vector128 scalar = Vector128.CreateScalarUnsafe(bits); From bf41da69b3b73f45f9c50c4fb92b183374ac77f1 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 26 Jul 2022 16:21:57 +0200 Subject: [PATCH 9/9] use magic bitmask provided by Tanner and port Avx2 code to Vector256 without big perf penalty --- .../src/System/Collections/BitArray.cs | 40 +++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/BitArray.cs b/src/libraries/System.Collections/src/System/Collections/BitArray.cs index ffc472648bd197..f368c91ace6a62 100644 --- a/src/libraries/System.Collections/src/System/Collections/BitArray.cs +++ b/src/libraries/System.Collections/src/System/Collections/BitArray.cs @@ -6,7 +6,6 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; -using System.Runtime.Intrinsics.X86; namespace System.Collections { @@ -804,35 +803,27 @@ public unsafe void CopyTo(Array array, int index) int[] thisArray = m_array; uint thisLength = (uint)m_length; - if (m_length < BitsPerInt32) + if (thisLength < BitsPerInt32) goto LessThan32; - if (Avx2.IsSupported) + if (Vector256.IsHardwareAccelerated) { - // The mask used when shuffling a single int into Vector128/256. - // On little endian machines, the lower 8 bits of int belong in the first byte, next lower 8 in the second and so on. - // We place the bytes that contain the bits to its respective byte so that we can mask out only the relevant bits later. - Vector128 lowerShuffleMask = Vector128.Create(0, 0x01010101_01010101).AsByte(); - Vector128 upperShuffleMask = Vector128.Create(0x02020202_02020202, 0x03030303_03030303).AsByte(); - - Vector256 shuffleMask = Vector256.Create(lowerShuffleMask, upperShuffleMask); Vector256 bitMask = Vector256.Create(0x80402010_08040201).AsByte(); Vector256 ones = Vector256.Create((byte)1); - fixed (bool* destination = &boolArray[index]) + ref byte destination = ref Unsafe.As(ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(boolArray), index)); + + for (; i <= thisLength - Vector256.Count; i += (uint)Vector256.Count) { - for (; i <= thisLength - Vector256.Count; i += (uint)Vector256.Count) - { - int bits = thisArray[i / (uint)BitsPerInt32]; - Vector256 scalar = Vector256.Create(bits); - Vector256 shuffled = Avx2.Shuffle(scalar.AsByte(), shuffleMask); - Vector256 extracted = Avx2.And(shuffled, bitMask); - - // The extracted bits can be anywhere between 0 and 255, so we normalise the value to either 0 or 1 - // to ensure compatibility with "C# bool" (0 for false, 1 for true, rest undefined) - Vector256 normalized = Avx2.Min(extracted, ones); - Avx.Store((byte*)destination + i, normalized); - } + int bits = thisArray[i / (uint)BitsPerInt32]; + Vector256 scalar = Vector256.Create(bits); + Vector256 shuffled = Vector256.Shuffle(scalar.AsByte(), Vector256.Create(0x0000_0000_0000_0000, 0x0101_0101_0101_0101, 0x12121212_12121212, 0x13131313_13131313).AsByte()); + Vector256 extracted = shuffled & bitMask; + + // The extracted bits can be anywhere between 0 and 255, so we normalise the value to either 0 or 1 + // to ensure compatibility with "C# bool" (0 for false, 1 for true, rest undefined) + Vector256 normalized = Vector256.Min(extracted, ones); + normalized.StoreUnsafe(ref destination, i); } } else if (Vector128.IsHardwareAccelerated) @@ -849,8 +840,7 @@ public unsafe void CopyTo(Array array, int index) int bits = thisArray[i / (uint)BitsPerInt32]; Vector128 scalar = Vector128.CreateScalarUnsafe(bits); - // the shuffle masks used below are the same as masks used by the Avx2 codepath above - // they need to be consts for optimal codegen (see https://github.com/dotnet/runtime/issues/72793) + // they masks need to be consts for optimal codegen (see https://github.com/dotnet/runtime/issues/72793) Vector128 shuffledLower = Vector128.Shuffle(scalar.AsByte(), Vector128.Create((byte)0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1)); Vector128 extractedLower = shuffledLower & bitMask128; Vector128 normalizedLower = Vector128.Min(extractedLower, ones);