From ce38d6aaef9abb33deafc85694bafd23c15024f3 Mon Sep 17 00:00:00 2001 From: Grant Dickinson Date: Mon, 4 Mar 2019 19:19:19 -0800 Subject: [PATCH 1/3] Update BitOps callsites --- .../System/Net/WebSockets/ManagedWebSocket.cs | 2 +- .../Internal/Utilities/BitArithmetic.cs | 31 ++----------------- .../Metadata/Ecma335/MetadataSizes.cs | 19 ++++++------ .../PortableExecutable/PEHeaderBuilder.cs | 14 ++++----- 4 files changed, 21 insertions(+), 45 deletions(-) diff --git a/src/Common/src/System/Net/WebSockets/ManagedWebSocket.cs b/src/Common/src/System/Net/WebSockets/ManagedWebSocket.cs index 86c5ba9e50ae..728788743adf 100644 --- a/src/Common/src/System/Net/WebSockets/ManagedWebSocket.cs +++ b/src/Common/src/System/Net/WebSockets/ManagedWebSocket.cs @@ -1262,7 +1262,7 @@ private static unsafe int ApplyMask(Span toMask, int mask, int maskIndex) Debug.Assert(maskIndex < sizeof(int)); int maskShift = maskIndex * 8; - int shiftedMask = (int)(((uint)mask >> maskShift) | ((uint)mask << (32 - maskShift))); + int shiftedMask = (int)BitOperations.RotateRight((uint)mask, maskShift); // Try to use SIMD. We can if the number of bytes we're trying to mask is at least as much // as the width of a vector and if the width is an even multiple of the mask. diff --git a/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs b/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs index bbf3d81370e8..9e7567a723d1 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs @@ -3,40 +3,15 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; +using System.Numerics; namespace System.Reflection.Internal { internal static class BitArithmetic { - internal static int CountBits(int v) - { - return CountBits(unchecked((uint)v)); - } - - internal static int CountBits(uint v) - { - unchecked - { - v = v - ((v >> 1) & 0x55555555u); - v = (v & 0x33333333u) + ((v >> 2) & 0x33333333u); - return (int)((v + (v >> 4) & 0xF0F0F0Fu) * 0x1010101u) >> 24; - } - } - - internal static int CountBits(ulong v) - { - const ulong Mask01010101 = 0x5555555555555555UL; - const ulong Mask00110011 = 0x3333333333333333UL; - const ulong Mask00001111 = 0x0F0F0F0F0F0F0F0FUL; - const ulong Mask00000001 = 0x0101010101010101UL; - v = v - ((v >> 1) & Mask01010101); - v = (v & Mask00110011) + ((v >> 2) & Mask00110011); - return (int)(unchecked(((v + (v >> 4)) & Mask00001111) * Mask00000001) >> 56); - } - internal static uint Align(uint position, uint alignment) { - Debug.Assert(CountBits(alignment) == 1); + Debug.Assert(BitOperations.PopCount(alignment) == 1); uint result = position & ~(alignment - 1); if (result == position) @@ -50,7 +25,7 @@ internal static uint Align(uint position, uint alignment) internal static int Align(int position, int alignment) { Debug.Assert(position >= 0 && alignment > 0); - Debug.Assert(CountBits(alignment) == 1); + Debug.Assert(BitOperations.PopCount(unchecked((uint)alignment)) == 1); int result = position & ~(alignment - 1); if (result == position) diff --git a/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs b/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs index 34c3df341d01..fc5d6bd9d46a 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs @@ -4,6 +4,7 @@ using System.Collections.Immutable; using System.Diagnostics; +using System.Numerics; using System.Reflection.Internal; namespace System.Reflection.Metadata.Ecma335 @@ -66,12 +67,12 @@ public sealed class MetadataSizes public ImmutableArray HeapSizes { get; } /// - /// Table row counts. + /// Table row counts. /// public ImmutableArray RowCounts { get; } /// - /// External table row counts. + /// External table row counts. /// public ImmutableArray ExternalRowCounts { get; } @@ -363,7 +364,7 @@ internal int MetadataHeaderSize MetadataVersionPaddedLength + // metadata version sizeof(ushort) + // storage header: reserved sizeof(ushort) + // stream count - (IsStandaloneDebugMetadata ? StandalonePdbStreamHeaderSize : 0) + + (IsStandaloneDebugMetadata ? StandalonePdbStreamHeaderSize : 0) + RegularStreamHeaderSizes + (IsEncDelta ? EncDeltaMarkerStreamHeaderSize : 0); } @@ -399,7 +400,7 @@ public int GetAlignedHeapSize(HeapIndex index) internal int CalculateTableStreamHeaderSize() { int result = sizeof(int) + // Reserved - sizeof(short) + // Version (major, minor) + sizeof(short) + // Version (major, minor) sizeof(byte) + // Heap index sizes sizeof(byte) + // Bit width of RowId sizeof(long) + // Valid table mask @@ -421,11 +422,11 @@ internal int CalculateTableStreamHeaderSize() internal int CalculateStandalonePdbStreamSize() { - int result = - PdbIdSize + // PDB ID - sizeof(int) + // EntryPoint - sizeof(long) + // ReferencedTypeSystemTables - BitArithmetic.CountBits(ExternalTablesMask) * sizeof(int); // External row counts + int result = + PdbIdSize + // PDB ID + sizeof(int) + // EntryPoint + sizeof(long) + // ReferencedTypeSystemTables + BitOperations.PopCount(ExternalTablesMask) * sizeof(int); // External row counts Debug.Assert(result % StreamAlignment == 0); return result; diff --git a/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs b/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs index 1dfdf0f01802..4cc3c8f24cba 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs @@ -2,7 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Reflection.Internal; +using System.Numerics; namespace System.Reflection.PortableExecutable { @@ -65,12 +65,12 @@ public PEHeaderBuilder( ulong sizeOfHeapReserve = 0x00100000, ulong sizeOfHeapCommit = 0x1000) { - if (fileAlignment < 512 || fileAlignment > 64 * 1024 || BitArithmetic.CountBits(fileAlignment) != 1) + if (fileAlignment < 512 || fileAlignment > 64 * 1024 || BitOperations.PopCount(unchecked((uint)fileAlignment)) != 1) { Throw.ArgumentOutOfRange(nameof(fileAlignment)); } - if (sectionAlignment < fileAlignment || BitArithmetic.CountBits(sectionAlignment) != 1) + if (sectionAlignment < fileAlignment || BitOperations.PopCount(unchecked((uint)sectionAlignment)) != 1) { Throw.ArgumentOutOfRange(nameof(sectionAlignment)); } @@ -98,7 +98,7 @@ public PEHeaderBuilder( public static PEHeaderBuilder CreateExecutableHeader() { - return new PEHeaderBuilder(imageCharacteristics : Characteristics.ExecutableImage); + return new PEHeaderBuilder(imageCharacteristics: Characteristics.ExecutableImage); } public static PEHeaderBuilder CreateLibraryHeader() @@ -111,8 +111,8 @@ public static PEHeaderBuilder CreateLibraryHeader() internal int ComputeSizeOfPEHeaders(int sectionCount) => PEBuilder.DosHeaderSize + PEHeaders.PESignatureSize + - CoffHeader.Size + - PEHeader.Size(Is32Bit) + + CoffHeader.Size + + PEHeader.Size(Is32Bit) + SectionHeader.Size * sectionCount; } -} \ No newline at end of file +} From 4102d83d9747f5688e2c20fa98630a36f5a96ad2 Mon Sep 17 00:00:00 2001 From: Grant Dickinson Date: Mon, 4 Mar 2019 19:35:08 -0800 Subject: [PATCH 2/3] Bugs --- .../System/Reflection/PortableExecutable/PEHeaderBuilder.cs | 1 + .../tests/Metadata/TagToTokenTests.cs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs b/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs index 4cc3c8f24cba..f5004317b541 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Numerics; +using System.Reflection.Internal; namespace System.Reflection.PortableExecutable { diff --git a/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs b/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs index fc5419a4d610..da60aaa52c0b 100644 --- a/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs +++ b/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; +using System.Numerics; using System.Reflection.Metadata.Ecma335; using Xunit; @@ -146,7 +147,7 @@ public void ValidateTagToTokenConversion() tag.Name + " did not round trip coded index -> handle -> coded index"); } - Assert.True(badTagCount == (1 << tag.GetNumberOfBits()) - BitArithmetic.CountBits((ulong)tag.GetTablesReferenced()), + Assert.True(badTagCount == (1 << tag.GetNumberOfBits()) - BitOperations.PopCount((ulong)tag.GetTablesReferenced()), tag.Name + " did not find the correct number of bad tags."); Assert.True(tablesNotUsed == 0, From 12725f76f5d2935e7e5410be49b0678053cb665e Mon Sep 17 00:00:00 2001 From: Grant Dickinson Date: Mon, 18 Mar 2019 11:19:33 -0700 Subject: [PATCH 3/3] Revert changes to System.Reflection due to NetStandard impact --- .../Internal/Utilities/BitArithmetic.cs | 31 +++++++++++++++++-- .../Metadata/Ecma335/MetadataSizes.cs | 9 +++--- .../PortableExecutable/PEHeaderBuilder.cs | 5 ++- .../tests/Metadata/TagToTokenTests.cs | 3 +- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs b/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs index 9e7567a723d1..bbf3d81370e8 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs @@ -3,15 +3,40 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; -using System.Numerics; namespace System.Reflection.Internal { internal static class BitArithmetic { + internal static int CountBits(int v) + { + return CountBits(unchecked((uint)v)); + } + + internal static int CountBits(uint v) + { + unchecked + { + v = v - ((v >> 1) & 0x55555555u); + v = (v & 0x33333333u) + ((v >> 2) & 0x33333333u); + return (int)((v + (v >> 4) & 0xF0F0F0Fu) * 0x1010101u) >> 24; + } + } + + internal static int CountBits(ulong v) + { + const ulong Mask01010101 = 0x5555555555555555UL; + const ulong Mask00110011 = 0x3333333333333333UL; + const ulong Mask00001111 = 0x0F0F0F0F0F0F0F0FUL; + const ulong Mask00000001 = 0x0101010101010101UL; + v = v - ((v >> 1) & Mask01010101); + v = (v & Mask00110011) + ((v >> 2) & Mask00110011); + return (int)(unchecked(((v + (v >> 4)) & Mask00001111) * Mask00000001) >> 56); + } + internal static uint Align(uint position, uint alignment) { - Debug.Assert(BitOperations.PopCount(alignment) == 1); + Debug.Assert(CountBits(alignment) == 1); uint result = position & ~(alignment - 1); if (result == position) @@ -25,7 +50,7 @@ internal static uint Align(uint position, uint alignment) internal static int Align(int position, int alignment) { Debug.Assert(position >= 0 && alignment > 0); - Debug.Assert(BitOperations.PopCount(unchecked((uint)alignment)) == 1); + Debug.Assert(CountBits(alignment) == 1); int result = position & ~(alignment - 1); if (result == position) diff --git a/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs b/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs index fc5d6bd9d46a..a5d15d64807b 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs @@ -4,7 +4,6 @@ using System.Collections.Immutable; using System.Diagnostics; -using System.Numerics; using System.Reflection.Internal; namespace System.Reflection.Metadata.Ecma335 @@ -423,10 +422,10 @@ internal int CalculateTableStreamHeaderSize() internal int CalculateStandalonePdbStreamSize() { int result = - PdbIdSize + // PDB ID - sizeof(int) + // EntryPoint - sizeof(long) + // ReferencedTypeSystemTables - BitOperations.PopCount(ExternalTablesMask) * sizeof(int); // External row counts + PdbIdSize + // PDB ID + sizeof(int) + // EntryPoint + sizeof(long) + // ReferencedTypeSystemTables + BitArithmetic.CountBits(ExternalTablesMask) * sizeof(int); // External row counts Debug.Assert(result % StreamAlignment == 0); return result; diff --git a/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs b/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs index cd0f6b89bca1..4cb10c2aa611 100644 --- a/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs +++ b/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Numerics; using System.Reflection.Internal; namespace System.Reflection.PortableExecutable @@ -66,12 +65,12 @@ public PEHeaderBuilder( ulong sizeOfHeapReserve = 0x00100000, ulong sizeOfHeapCommit = 0x1000) { - if (fileAlignment < 512 || fileAlignment > 64 * 1024 || BitOperations.PopCount(unchecked((uint)fileAlignment)) != 1) + if (fileAlignment < 512 || fileAlignment > 64 * 1024 || BitArithmetic.CountBits(fileAlignment) != 1) { Throw.ArgumentOutOfRange(nameof(fileAlignment)); } - if (sectionAlignment < fileAlignment || BitOperations.PopCount(unchecked((uint)sectionAlignment)) != 1) + if (sectionAlignment < fileAlignment || BitArithmetic.CountBits(sectionAlignment) != 1) { Throw.ArgumentOutOfRange(nameof(sectionAlignment)); } diff --git a/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs b/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs index da60aaa52c0b..fc5419a4d610 100644 --- a/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs +++ b/src/System.Reflection.Metadata/tests/Metadata/TagToTokenTests.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Linq; -using System.Numerics; using System.Reflection.Metadata.Ecma335; using Xunit; @@ -147,7 +146,7 @@ public void ValidateTagToTokenConversion() tag.Name + " did not round trip coded index -> handle -> coded index"); } - Assert.True(badTagCount == (1 << tag.GetNumberOfBits()) - BitOperations.PopCount((ulong)tag.GetTablesReferenced()), + Assert.True(badTagCount == (1 << tag.GetNumberOfBits()) - BitArithmetic.CountBits((ulong)tag.GetTablesReferenced()), tag.Name + " did not find the correct number of bad tags."); Assert.True(tablesNotUsed == 0,