Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Eliminate redundant test instruction#53214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
kunalspathak merged 10 commits intodotnet:mainfromkunalspathak:ins_test
Jun 5, 2021

Conversation

@kunalspathak
Copy link
Contributor

@kunalspathakkunalspathak commentedMay 25, 2021•
edited
Loading

Eliminatetest instruction if previous instruction writes toZF flag. Thanks to@tannergooding for updating the instruction table with flags that it reads/writes. I use that information in the code added by@nathan-moore
in#38586. It covers other remaining cases forJE/JNE, specially around bit operation instructions. In future, we would expand to cover other flags/Jcc instructions.

Also included, minimal natvis to display instructions.

Contributes to#53053.

Below is the asmdiff summary. 1-2 method regression comes because of loop alignment adjustment.

Unix x64 benchmarks

Summary of Code Size diffs:(Lower is better)Total bytes of base: 51949Total bytes of diff: 51714Total bytes of delta: -235 (-0.45% of base)    diff is an improvement.
Detail diffs
Top file improvements (bytes):          -9 : 25257.dasm (-0.58% of base)          -8 : 27057.dasm (-10.67% of base)          -7 : 1968.dasm (-0.49% of base)          -6 : 12616.dasm (-1.66% of base)          -6 : 3692.dasm (-2.13% of base)          -6 : 12366.dasm (-2.24% of base)          -6 : 29887.dasm (-0.52% of base)          -6 : 2653.dasm (-0.59% of base)          -6 : 5945.dasm (-1.49% of base)          -5 : 601.dasm (-1.34% of base)          -5 : 6613.dasm (-0.61% of base)          -4 : 3506.dasm (-0.34% of base)          -4 : 547.dasm (-0.32% of base)          -4 : 14827.dasm (-1.24% of base)          -4 : 9339.dasm (-1.18% of base)          -4 : 7054.dasm (-0.44% of base)          -4 : 6110.dasm (-1.20% of base)          -4 : 12628.dasm (-1.21% of base)          -4 : 25719.dasm (-1.74% of base)          -3 : 2553.dasm (-2.56% of base)77 total files with Code Size differences (77 improved, 0 regressed), 3 unchanged.Top method improvements (bytes):          -9 (-0.58% of base) : 25257.dasm - System.Numerics.BigIntegerCalculator:Multiply(long,int,long,int,long,int)          -8 (-10.67% of base) : 27057.dasm - V8.Crypto.BigInteger:nbits(int):int:this          -7 (-0.49% of base) : 1968.dasm - Internal.Cryptography.Pal.UnixPkcs12Reader:Decrypt(System.ReadOnlySpan`1[Char],System.ReadOnlyMemory`1[Byte]):this          -6 (-1.66% of base) : 12616.dasm - EscaperImplementation:EncodeUtf8(System.Text.Rune,System.Span`1[Byte]):int:this          -6 (-2.13% of base) : 3692.dasm - Enumerator[__Canon][System.__Canon]:MoveNext():bool:this          -6 (-2.24% of base) : 12366.dasm - StackEnumerator:MoveNext():bool:this          -6 (-0.52% of base) : 29887.dasm - System.Numerics.BigIntegerCalculator:Square(long,int,long,int)          -6 (-0.59% of base) : 2653.dasm - WorkingChain:VerifyCallback(int,long):int:this          -6 (-1.49% of base) : 5945.dasm - EscaperImplementation:EncodeUtf16(System.Text.Rune,System.Span`1[Char]):int:this          -5 (-1.34% of base) : 601.dasm - System.StubHelpers.CSTRMarshaler:ConvertToNative(int,System.String,long):long          -5 (-0.61% of base) : 6613.dasm - ProtoBuf.ProtoWriter:EndSubItem(ProtoBuf.SubItemToken,ProtoBuf.ProtoWriter,int)          -4 (-0.34% of base) : 3506.dasm - System.Array:Copy(System.Array,int,System.Array,int,int,bool)          -4 (-0.32% of base) : 547.dasm - System.Version:ParseVersion(System.ReadOnlySpan`1[Char],bool):System.Version          -4 (-1.24% of base) : 14827.dasm - Node[__Canon][System.__Canon]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[__Canon],int,int):Node[__Canon]          -4 (-1.18% of base) : 9339.dasm - Node[Int32,Int32][System.Int32,System.Int32]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[KeyValuePair`2],int,int):Node[Int32,Int32]          -4 (-0.44% of base) : 7054.dasm - System.Array:Sort(System.Array,System.Array,int,int,System.Collections.IComparer)          -4 (-1.20% of base) : 6110.dasm - Node[__Canon,__Canon][System.__Canon,System.__Canon]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[KeyValuePair`2],int,int):Node[__Canon,__Canon]          -4 (-1.21% of base) : 12628.dasm - Node[Int32][System.Int32]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[Int32],int,int):Node[Int32]          -4 (-1.74% of base) : 25719.dasm - Enumerator[Int32][System.Int32]:MoveNext():bool:this          -3 (-2.56% of base) : 2553.dasm - System.String:InitializeProbabilisticMap(long,System.ReadOnlySpan`1[Char])Top method improvements (percentages):          -8 (-10.67% of base) : 27057.dasm - V8.Crypto.BigInteger:nbits(int):int:this          -2 (-4.76% of base) : 3945.dasm - System.Text.RegularExpressions.Regex:ValidateOptions(int)          -2 (-3.51% of base) : 3720.dasm - Newtonsoft.Json.JsonTextWriter:WriteIntegerValue(int):this          -2 (-3.39% of base) : 18305.dasm - Builder[__Canon][System.__Canon]:AddRange(System.Collections.Immutable.ImmutableArray`1[__Canon],int):this          -3 (-3.09% of base) : 2185.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 2050.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 2042.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -2 (-2.63% of base) : 264.dasm - System.ConsolePal:TryGetCachedCursorPosition(byref,byref):bool          -3 (-2.56% of base) : 2553.dasm - System.String:InitializeProbabilisticMap(long,System.ReadOnlySpan`1[Char])          -2 (-2.41% of base) : 663.dasm - Builder[SectionHeader][System.Reflection.PortableExecutable.SectionHeader]:.ctor(int):this          -2 (-2.25% of base) : 17895.dasm - Builder[__Canon][System.__Canon]:.ctor(int):this          -6 (-2.24% of base) : 12366.dasm - StackEnumerator:MoveNext():bool:this          -6 (-2.13% of base) : 3692.dasm - Enumerator[__Canon][System.__Canon]:MoveNext():bool:this          -3 (-1.82% of base) : 12679.dasm - System.Convert:ToBase64_CalculateAndValidateOutputLength(int,bool):int          -4 (-1.74% of base) : 25719.dasm - Enumerator[Int32][System.Int32]:MoveNext():bool:this          -3 (-1.72% of base) : 16577.dasm - ProtoBuf.ProtoWriter:WriteUInt64Variant(long,ProtoBuf.ProtoWriter)          -3 (-1.71% of base) : 27563.dasm - System.Collections.CreateAddAndClear`1[Int32][System.Int32]:Span():System.Span`1[Int32]:this          -6 (-1.66% of base) : 12616.dasm - EscaperImplementation:EncodeUtf8(System.Text.Rune,System.Span`1[Byte]):int:this          -2 (-1.54% of base) : 15873.dasm - System.Array:GetValue(int):System.Object:this          -6 (-1.49% of base) : 5945.dasm - EscaperImplementation:EncodeUtf16(System.Text.Rune,System.Span`1[Char]):int:this77 total methods with Code Size differences (77 improved, 0 regressed), 3 unchanged.

Unix x64 libraries

Summary of Code Size diffs:(Lower is better)Total bytes of base: 215248Total bytes of diff: 214330Total bytes of delta: -918 (-0.43% of base)    diff is an improvement.
Detail diffs
Top file regressions (bytes):           1 : 180519.dasm (0.08% of base)Top file improvements (bytes):         -16 : 68564.dasm (-1.34% of base)         -15 : 68639.dasm (-1.19% of base)         -13 : 16761.dasm (-1.59% of base)         -13 : 113493.dasm (-1.65% of base)         -10 : 140680.dasm (-2.67% of base)          -9 : 213180.dasm (-0.58% of base)          -8 : 211610.dasm (-3.01% of base)          -7 : 185349.dasm (-0.39% of base)          -7 : 16262.dasm (-0.42% of base)          -7 : 6949.dasm (-0.39% of base)          -7 : 17424.dasm (-0.77% of base)          -7 : 178931.dasm (-0.39% of base)          -7 : 7010.dasm (-0.66% of base)          -7 : 8427.dasm (-0.66% of base)          -7 : 10713.dasm (-0.49% of base)          -6 : 10524.dasm (-0.57% of base)          -6 : 181265.dasm (-2.13% of base)          -6 : 69495.dasm (-0.16% of base)          -6 : 211486.dasm (-1.30% of base)          -6 : 211556.dasm (-1.37% of base)313 total files with Code Size differences (312 improved, 1 regressed), 4 unchanged.Top method regressions (bytes):           1 ( 0.08% of base) : 180519.dasm - System.Collections.Generic.SortedList`2[__Canon,Nullable`1][System.__Canon,System.Nullable`1[System.Int32]]:System.Collections.ICollection.CopyTo(System.Array,int):thisTop method improvements (bytes):         -16 (-1.34% of base) : 68564.dasm - HashCompare:GenericEqualityArbArray(bool,System.Collections.IEqualityComparer,System.Array,System.Array):bool         -15 (-1.19% of base) : 68639.dasm - HashCompare:GenericComparisonArbArrayWithComparer(GenericComparer,System.Array,System.Array):int         -13 (-1.59% of base) : 16761.dasm - Microsoft.VisualBasic.CompilerServices.Utils:CopyArray(System.Array,System.Array):System.Array         -13 (-1.65% of base) : 113493.dasm - Microsoft.VisualBasic.CompilerServices.Utils:CopyArray(System.Array,System.Array):System.Array         -10 (-2.67% of base) : 140680.dasm - Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator:StackMergeType(Microsoft.CodeAnalysis.CSharp.BoundExpression):Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol:this          -9 (-0.58% of base) : 213180.dasm - System.Numerics.BigIntegerCalculator:Multiply(long,int,long,int,long,int)          -8 (-3.01% of base) : 211610.dasm - Node[Byte][System.Byte]:Sort(int,int,System.Collections.Generic.IComparer`1[Byte]):Node[Byte]:this          -7 (-0.39% of base) : 185349.dasm - Xunit.Sdk.AssertEqualityComparer`1[__Canon][System.__Canon]:Equals(System.__Canon,System.__Canon):bool:this          -7 (-0.42% of base) : 16262.dasm - Microsoft.VisualBasic.CompilerServices.LikeOperator:BuildPatternGroups(System.String,int,byref,Microsoft.VisualBasic.CompilerServices.LikeOperator+LigatureInfo[],System.String,int,byref,Microsoft.VisualBasic.CompilerServices.LikeOperator+LigatureInfo[],byref,byref,System.Globalization.CompareInfo,int,byref)          -7 (-0.39% of base) : 6949.dasm - Xunit.Sdk.AssertEqualityComparer`1[__Canon][System.__Canon]:Equals(System.__Canon,System.__Canon):bool:this          -7 (-0.77% of base) : 17424.dasm - Microsoft.VisualBasic.Information:TypeName(System.Object):System.String          -7 (-0.39% of base) : 178931.dasm - Xunit.Sdk.AssertEqualityComparer`1[__Canon][System.__Canon]:Equals(System.__Canon,System.__Canon):bool:this          -7 (-0.66% of base) : 7010.dasm - ArraySerializer:Serialize(Xunit.Abstractions.IXunitSerializationInfo):this          -7 (-0.66% of base) : 8427.dasm - ArraySerializer:Serialize(Xunit.Abstractions.IXunitSerializationInfo):this          -7 (-0.49% of base) : 10713.dasm - Internal.Cryptography.Pal.UnixPkcs12Reader:Decrypt(System.ReadOnlySpan`1[Char],System.ReadOnlyMemory`1[Byte]):this          -6 (-0.57% of base) : 10524.dasm - WorkingChain:VerifyCallback(int,long):int:this          -6 (-2.13% of base) : 181265.dasm - Enumerator[__Canon][System.__Canon]:MoveNext():bool:this          -6 (-0.16% of base) : 69495.dasm - Microsoft.Diagnostics.Symbols.SymbolReader:GenerateNGenSymbolsForModule(System.String,System.String):System.String:this          -6 (-1.30% of base) : 211486.dasm - Node[__Canon][System.__Canon]:CopyTo(int,System.__Canon[],int,int):this          -6 (-1.37% of base) : 211556.dasm - Node[Byte][System.Byte]:CopyTo(int,System.Byte[],int,int):thisTop method regressions (percentages):           1 ( 0.08% of base) : 180519.dasm - System.Collections.Generic.SortedList`2[__Canon,Nullable`1][System.__Canon,System.Nullable`1[System.Int32]]:System.Collections.ICollection.CopyTo(System.Array,int):thisTop method improvements (percentages):          -2 (-4.76% of base) : 26.dasm - System.Text.RegularExpressions.Regex:ValidateOptions(int)          -3 (-3.85% of base) : 1256.dasm - System.Span`1[Vector`1][System.Numerics.Vector`1[System.Single]]:Clear():this          -3 (-3.61% of base) : 192172.dasm - System.Runtime.Caching.CacheMemoryMonitor:SetLimit(int):this          -2 (-3.51% of base) : 19211.dasm - Newtonsoft.Json.JsonTextWriter:WriteIntegerValue(int):this          -2 (-3.39% of base) : 209961.dasm - Builder[Byte][System.Byte]:AddRange(System.Collections.Immutable.ImmutableArray`1[Byte],int):this          -2 (-3.39% of base) : 209917.dasm - Builder[__Canon][System.__Canon]:AddRange(System.Collections.Immutable.ImmutableArray`1[__Canon],int):this          -4 (-3.28% of base) : 211329.dasm - Builder[Byte][System.Byte]:GetRange(int,int):System.Collections.Immutable.ImmutableList`1[Byte]:this          -3 (-3.12% of base) : 181290.dasm - System.Collections.Generic.BitHelper:.ctor(System.Span`1[Int32],bool):this          -3 (-3.09% of base) : 184907.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 171807.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 156177.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 183721.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 219435.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 10837.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -3 (-3.09% of base) : 208929.dasm - System.Security.Cryptography.CryptoPool:Return(System.Byte[],int)          -8 (-3.01% of base) : 211610.dasm - Node[Byte][System.Byte]:Sort(int,int,System.Collections.Generic.IComparer`1[Byte]):Node[Byte]:this          -4 (-2.82% of base) : 211035.dasm - System.Collections.Immutable.ImmutableList`1[Byte][System.Byte]:GetRange(int,int):System.Collections.Immutable.ImmutableList`1[Byte]:this          -4 (-2.82% of base) : 211092.dasm - System.Collections.Immutable.ImmutableList`1[Byte][System.Byte]:Sort(int,int,System.Collections.Generic.IComparer`1[Byte]):System.Collections.Immutable.ImmutableList`1[Byte]:this          -4 (-2.82% of base) : 211013.dasm - System.Collections.Immutable.ImmutableList`1[__Canon][System.__Canon]:Sort(int,int,System.Collections.Generic.IComparer`1[__Canon]):System.Collections.Immutable.ImmutableList`1[__Canon]:this          -2 (-2.74% of base) : 216899.dasm - System.Diagnostics.TraceListener:set_TraceOutputOptions(int):this313 total methods with Code Size differences (312 improved, 1 regressed), 4 unchanged.

Windows x64 asp.net

Summary of Code Size diffs:(Lower is better)Total bytes of base: 2998Total bytes of diff: 2966Total bytes of delta: -32 (-1.07% of base)    diff is an improvement.
Detail diffs
Top file improvements (bytes):          -5 : 27000.dasm (-0.46% of base)          -5 : 41342.dasm (-0.46% of base)          -3 : 8711.dasm (-2.52% of base)          -3 : 1814.dasm (-2.52% of base)          -2 : 13113.dasm (-8.70% of base)          -2 : 20842.dasm (-1.92% of base)          -2 : 20822.dasm (-8.70% of base)          -2 : 27489.dasm (-1.92% of base)          -2 : 40932.dasm (-1.82% of base)          -2 : 1278.dasm (-2.20% of base)          -2 : 39986.dasm (-8.70% of base)          -2 : 26882.dasm (-1.82% of base)12 total files with Code Size differences (12 improved, 0 regressed), 0 unchanged.Top method improvements (bytes):          -5 (-0.46% of base) : 27000.dasm - Array:Copy(Array,int,Array,int,int,bool)          -5 (-0.46% of base) : 41342.dasm - Array:Copy(Array,int,Array,int,int,bool)          -3 (-2.52% of base) : 8711.dasm - String:InitializeProbabilisticMap(long,ReadOnlySpan`1)          -3 (-2.52% of base) : 1814.dasm - String:InitializeProbabilisticMap(long,ReadOnlySpan`1)          -2 (-8.70% of base) : 13113.dasm - Array:get_Rank():int:this          -2 (-1.92% of base) : 20842.dasm - Array:CopyTo(Array,int):this          -2 (-8.70% of base) : 20822.dasm - Array:get_Rank():int:this          -2 (-1.92% of base) : 27489.dasm - Array:CopyTo(Array,int):this          -2 (-1.82% of base) : 40932.dasm - Array:SetValue(Object,int):this          -2 (-2.20% of base) : 1278.dasm - Builder:.ctor(int):this          -2 (-8.70% of base) : 39986.dasm - Array:get_Rank():int:this          -2 (-1.82% of base) : 26882.dasm - Array:SetValue(Object,int):thisTop method improvements (percentages):          -2 (-8.70% of base) : 13113.dasm - Array:get_Rank():int:this          -2 (-8.70% of base) : 20822.dasm - Array:get_Rank():int:this          -2 (-8.70% of base) : 39986.dasm - Array:get_Rank():int:this          -3 (-2.52% of base) : 8711.dasm - String:InitializeProbabilisticMap(long,ReadOnlySpan`1)          -3 (-2.52% of base) : 1814.dasm - String:InitializeProbabilisticMap(long,ReadOnlySpan`1)          -2 (-2.20% of base) : 1278.dasm - Builder:.ctor(int):this          -2 (-1.92% of base) : 20842.dasm - Array:CopyTo(Array,int):this          -2 (-1.92% of base) : 27489.dasm - Array:CopyTo(Array,int):this          -2 (-1.82% of base) : 40932.dasm - Array:SetValue(Object,int):this          -2 (-1.82% of base) : 26882.dasm - Array:SetValue(Object,int):this          -5 (-0.46% of base) : 27000.dasm - Array:Copy(Array,int,Array,int,int,bool)          -5 (-0.46% of base) : 41342.dasm - Array:Copy(Array,int,Array,int,int,bool)12 total methods with Code Size differences (12 improved, 0 regressed), 0 unchanged.

Windows x64 benchmarks

Summary of Code Size diffs:(Lower is better)Total bytes of base: 48402Total bytes of diff: 48183Total bytes of delta: -219 (-0.45% of base)    diff is an improvement.
Detail diffs
Top file improvements (bytes):         -13 : 1696.dasm (-1.64% of base)         -12 : 9610.dasm (-0.75% of base)          -9 : 16952.dasm (-0.57% of base)          -8 : 24247.dasm (-11.43% of base)          -6 : 12375.dasm (-1.69% of base)          -6 : 16949.dasm (-0.51% of base)          -6 : 10805.dasm (-1.49% of base)          -5 : 1255.dasm (-0.43% of base)          -5 : 13017.dasm (-1.04% of base)          -4 : 13977.dasm (-1.28% of base)          -4 : 12161.dasm (-1.67% of base)          -4 : 8104.dasm (-1.25% of base)          -4 : 3518.dasm (-0.47% of base)          -4 : 24436.dasm (-1.69% of base)          -4 : 457.dasm (-0.30% of base)          -4 : 12151.dasm (-1.31% of base)          -4 : 3082.dasm (-1.52% of base)          -4 : 4465.dasm (-1.17% of base)          -3 : 21758.dasm (-0.57% of base)          -3 : 11942.dasm (-1.84% of base)69 total files with Code Size differences (69 improved, 0 regressed), 3 unchanged.Top method improvements (bytes):         -13 (-1.64% of base) : 1696.dasm - ProtoBuf.ProtoWriter:EndSubItem(ProtoBuf.SubItemToken,ProtoBuf.ProtoWriter,int)         -12 (-0.75% of base) : 9610.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this          -9 (-0.57% of base) : 16952.dasm - System.Numerics.BigIntegerCalculator:Multiply(long,int,long,int,long,int)          -8 (-11.43% of base) : 24247.dasm - V8.Crypto.BigInteger:nbits(int):int:this          -6 (-1.69% of base) : 12375.dasm - EscaperImplementation:EncodeUtf8(System.Text.Rune,System.Span`1[Byte]):int:this          -6 (-0.51% of base) : 16949.dasm - System.Numerics.BigIntegerCalculator:Square(long,int,long,int)          -6 (-1.49% of base) : 10805.dasm - EscaperImplementation:EncodeUtf16(System.Text.Rune,System.Span`1[Char]):int:this          -5 (-0.43% of base) : 1255.dasm - System.Array:Copy(System.Array,int,System.Array,int,int,bool)          -5 (-1.04% of base) : 13017.dasm - System.StubHelpers.CSTRMarshaler:ConvertToNative(int,System.String,long):long          -4 (-1.28% of base) : 13977.dasm - Node[__Canon][System.__Canon]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[__Canon],int,int):Node[__Canon]          -4 (-1.67% of base) : 12161.dasm - StackEnumerator:MoveNext():bool:this          -4 (-1.25% of base) : 8104.dasm - Node[Int32,Int32][System.Int32,System.Int32]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[KeyValuePair`2],int,int):Node[Int32,Int32]          -4 (-0.47% of base) : 3518.dasm - System.Array:Sort(System.Array,System.Array,int,int,System.Collections.IComparer)          -4 (-1.69% of base) : 24436.dasm - Enumerator[Int32][System.Int32]:MoveNext():bool:this          -4 (-0.30% of base) : 457.dasm - System.Version:ParseVersion(System.ReadOnlySpan`1[Char],bool):System.Version          -4 (-1.31% of base) : 12151.dasm - Node[Int32][System.Int32]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[Int32],int,int):Node[Int32]          -4 (-1.52% of base) : 3082.dasm - Enumerator[__Canon][System.__Canon]:MoveNext():bool:this          -4 (-1.17% of base) : 4465.dasm - Node[__Canon,__Canon][System.__Canon,System.__Canon]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[KeyValuePair`2],int,int):Node[__Canon,__Canon]          -3 (-0.57% of base) : 21758.dasm - System.Collections.BitArray:LeftShift(int):System.Collections.BitArray:this          -3 (-1.84% of base) : 11942.dasm - System.Convert:ToBase64_CalculateAndValidateOutputLength(int,bool):intTop method improvements (percentages):          -8 (-11.43% of base) : 24247.dasm - V8.Crypto.BigInteger:nbits(int):int:this          -2 (-5.56% of base) : 2003.dasm - System.Text.RegularExpressions.Regex:ValidateOptions(int)          -3 (-5.56% of base) : 3107.dasm - Newtonsoft.Json.JsonTextWriter:WriteIntegerValue(int):this          -2 (-3.17% of base) : 17638.dasm - Builder[__Canon][System.__Canon]:AddRange(System.Collections.Immutable.ImmutableArray`1[__Canon],int):this          -3 (-2.52% of base) : 3614.dasm - System.String:InitializeProbabilisticMap(long,System.ReadOnlySpan`1[Char])          -2 (-2.25% of base) : 17219.dasm - Builder[__Canon][System.__Canon]:.ctor(int):this          -2 (-2.20% of base) : 10383.dasm - Builder[SectionHeader][System.Reflection.PortableExecutable.SectionHeader]:.ctor(int):this          -3 (-1.84% of base) : 11942.dasm - System.Convert:ToBase64_CalculateAndValidateOutputLength(int,bool):int          -4 (-1.69% of base) : 24436.dasm - Enumerator[Int32][System.Int32]:MoveNext():bool:this          -3 (-1.69% of base) : 15446.dasm - ProtoBuf.ProtoWriter:WriteUInt64Variant(long,ProtoBuf.ProtoWriter)          -6 (-1.69% of base) : 12375.dasm - EscaperImplementation:EncodeUtf8(System.Text.Rune,System.Span`1[Byte]):int:this          -3 (-1.68% of base) : 25574.dasm - System.Collections.CreateAddAndClear`1[Int32][System.Int32]:Span():System.Span`1[Int32]:this          -4 (-1.67% of base) : 12161.dasm - StackEnumerator:MoveNext():bool:this         -13 (-1.64% of base) : 1696.dasm - ProtoBuf.ProtoWriter:EndSubItem(ProtoBuf.SubItemToken,ProtoBuf.ProtoWriter,int)          -4 (-1.52% of base) : 3082.dasm - Enumerator[__Canon][System.__Canon]:MoveNext():bool:this          -6 (-1.49% of base) : 10805.dasm - EscaperImplementation:EncodeUtf16(System.Text.Rune,System.Span`1[Char]):int:this          -2 (-1.43% of base) : 14840.dasm - System.Array:GetValue(int):System.Object:this          -2 (-1.37% of base) : 1253.dasm - System.Array:CopyTo(System.Array,int):this          -2 (-1.34% of base) : 3955.dasm - System.Array:SetValue(System.Object,int):this          -4 (-1.31% of base) : 12151.dasm - Node[Int32][System.Int32]:NodeTreeFromList(System.Collections.Immutable.IOrderedCollection`1[Int32],int,int):Node[Int32]69 total methods with Code Size differences (69 improved, 0 regressed), 3 unchanged.

Windows x64 libraries

Summary of Code Size diffs:(Lower is better)Total bytes of base: 229571Total bytes of diff: 228587Total bytes of delta: -984 (-0.43% of base)    diff is an improvement.
Detail diffs
Top file regressions (bytes):           4 : 155980.dasm (0.25% of base)           2 : 145095.dasm (0.11% of base)Top file improvements (bytes):         -16 : 14411.dasm (-1.33% of base)         -16 : 14336.dasm (-1.41% of base)         -10 : 42804.dasm (-2.79% of base)         -10 : 69883.dasm (-1.37% of base)         -10 : 145593.dasm (-1.31% of base)          -9 : 210162.dasm (-0.56% of base)          -9 : 153945.dasm (-1.88% of base)          -9 : 154015.dasm (-1.92% of base)          -7 : 235778.dasm (-0.70% of base)          -7 : 145225.dasm (-1.25% of base)          -7 : 232676.dasm (-0.39% of base)          -7 : 233271.dasm (-0.39% of base)          -7 : 234081.dasm (-0.39% of base)          -7 : 234139.dasm (-0.70% of base)          -6 : 117380.dasm (-1.45% of base)          -6 : 20332.dasm (-0.48% of base)          -6 : 212833.dasm (-1.01% of base)          -6 : 219142.dasm (-1.69% of base)          -6 : 219143.dasm (-1.49% of base)          -6 : 210159.dasm (-0.53% of base)349 total files with Code Size differences (347 improved, 2 regressed), 3 unchanged.Top method regressions (bytes):           4 ( 0.25% of base) : 155980.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this           2 ( 0.11% of base) : 145095.dasm - Microsoft.VisualBasic.CompilerServices.LikeOperator:BuildPatternGroups(System.String,int,byref,Microsoft.VisualBasic.CompilerServices.LikeOperator+LigatureInfo[],System.String,int,byref,Microsoft.VisualBasic.CompilerServices.LikeOperator+LigatureInfo[],byref,byref,System.Globalization.CompareInfo,int,byref)Top method improvements (bytes):         -16 (-1.33% of base) : 14411.dasm - HashCompare:GenericComparisonArbArrayWithComparer(GenericComparer,System.Array,System.Array):int         -16 (-1.41% of base) : 14336.dasm - HashCompare:GenericEqualityArbArray(bool,System.Collections.IEqualityComparer,System.Array,System.Array):bool         -10 (-2.79% of base) : 42804.dasm - Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator:StackMergeType(Microsoft.CodeAnalysis.CSharp.BoundExpression):Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol:this         -10 (-1.37% of base) : 69883.dasm - Microsoft.VisualBasic.CompilerServices.Utils:CopyArray(System.Array,System.Array):System.Array         -10 (-1.31% of base) : 145593.dasm - Microsoft.VisualBasic.CompilerServices.Utils:CopyArray(System.Array,System.Array):System.Array          -9 (-0.56% of base) : 210162.dasm - System.Numerics.BigIntegerCalculator:Multiply(long,int,long,int,long,int)          -9 (-1.88% of base) : 153945.dasm - Node[__Canon][System.__Canon]:CopyTo(int,System.__Canon[],int,int):this          -9 (-1.92% of base) : 154015.dasm - Node[Byte][System.Byte]:CopyTo(int,System.Byte[],int,int):this          -7 (-0.70% of base) : 235778.dasm - ArraySerializer:Serialize(Xunit.Abstractions.IXunitSerializationInfo):this          -7 (-1.25% of base) : 145225.dasm - Microsoft.VisualBasic.CompilerServices.ObjectType:GetWidestType(System.Object,System.Object,bool):int          -7 (-0.39% of base) : 232676.dasm - Xunit.Sdk.AssertEqualityComparer`1[__Canon][System.__Canon]:Equals(System.__Canon,System.__Canon):bool:this          -7 (-0.39% of base) : 233271.dasm - Xunit.Sdk.AssertEqualityComparer`1[__Canon][System.__Canon]:Equals(System.__Canon,System.__Canon):bool:this          -7 (-0.39% of base) : 234081.dasm - Xunit.Sdk.AssertEqualityComparer`1[__Canon][System.__Canon]:Equals(System.__Canon,System.__Canon):bool:this          -7 (-0.70% of base) : 234139.dasm - ArraySerializer:Serialize(Xunit.Abstractions.IXunitSerializationInfo):this          -6 (-1.45% of base) : 117380.dasm - MS.Internal.Xml.Cache.XPathDocumentNavigator:get_UniqueId():System.String:this          -6 (-0.48% of base) : 20332.dasm - System.Collections.Generic.HashSet`1[Byte][System.Byte]:SymmetricExceptWithEnumerable(System.Collections.Generic.IEnumerable`1[Byte]):this          -6 (-1.01% of base) : 212833.dasm - System.Security.Cryptography.CngPkcs8:RewriteEncryptedPkcs8PrivateKey(System.Security.Cryptography.AsymmetricAlgorithm,System.ReadOnlySpan`1[Char],System.Security.Cryptography.PbeParameters):System.Formats.Asn1.AsnWriter          -6 (-1.69% of base) : 219142.dasm - EscaperImplementation:EncodeUtf8(System.Text.Rune,System.Span`1[Byte]):int:this          -6 (-1.49% of base) : 219143.dasm - EscaperImplementation:EncodeUtf16(System.Text.Rune,System.Span`1[Char]):int:this          -6 (-0.53% of base) : 210159.dasm - System.Numerics.BigIntegerCalculator:Square(long,int,long,int)Top method regressions (percentages):           4 ( 0.25% of base) : 155980.dasm - System.Collections.BitArray:CopyTo(System.Array,int):this           2 ( 0.11% of base) : 145095.dasm - Microsoft.VisualBasic.CompilerServices.LikeOperator:BuildPatternGroups(System.String,int,byref,Microsoft.VisualBasic.CompilerServices.LikeOperator+LigatureInfo[],System.String,int,byref,Microsoft.VisualBasic.CompilerServices.LikeOperator+LigatureInfo[],byref,byref,System.Globalization.CompareInfo,int,byref)Top method improvements (percentages):          -2 (-16.67% of base) : 228089.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:IsValidReg(int):bool          -2 (-16.67% of base) : 228088.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:IsLowReg(int):bool          -2 (-12.50% of base) : 228087.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:IsBitNumOverflow(int,ubyte):bool          -2 (-5.56% of base) : 25.dasm - System.Text.RegularExpressions.Regex:ValidateOptions(int)          -3 (-5.56% of base) : 101889.dasm - Newtonsoft.Json.JsonTextWriter:WriteIntegerValue(int):this          -3 (-4.41% of base) : 15708.dasm - System.Span`1[Vector`1][System.Numerics.Vector`1[System.Single]]:Clear():this          -4 (-3.92% of base) : 228090.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:EmitMOV(int,int):this          -4 (-3.92% of base) : 228091.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:EmitCMP(int,int):this          -4 (-3.92% of base) : 228092.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:EmitADD(int,int):this          -4 (-3.92% of base) : 228093.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:EmitSUB(int,int):this          -3 (-3.85% of base) : 209659.dasm - System.Runtime.Caching.CacheMemoryMonitor:SetLimit(int):this          -4 (-3.67% of base) : 228099.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:EmitLDR(int,int):this          -3 (-3.33% of base) : 157420.dasm - System.Collections.Generic.BitHelper:.ctor(System.Span`1[Int32],bool):this          -2 (-3.17% of base) : 152408.dasm - Builder[Byte][System.Byte]:AddRange(System.Collections.Immutable.ImmutableArray`1[Byte],int):this          -2 (-3.17% of base) : 152364.dasm - Builder[__Canon][System.__Canon]:AddRange(System.Collections.Immutable.ImmutableArray`1[__Canon],int):this          -4 (-3.12% of base) : 153788.dasm - Builder[Byte][System.Byte]:GetRange(int,int):System.Collections.Immutable.ImmutableList`1[Byte]:this          -4 (-2.94% of base) : 228098.dasm - ILCompiler.DependencyAnalysis.ARM.ARMEmitter:EmitMOV(int,int):this          -4 (-2.90% of base) : 153494.dasm - System.Collections.Immutable.ImmutableList`1[Byte][System.Byte]:GetRange(int,int):System.Collections.Immutable.ImmutableList`1[Byte]:this          -4 (-2.88% of base) : 153472.dasm - System.Collections.Immutable.ImmutableList`1[__Canon][System.__Canon]:Sort(int,int,System.Collections.Generic.IComparer`1[__Canon]):System.Collections.Immutable.ImmutableList`1[__Canon]:this          -4 (-2.88% of base) : 153551.dasm - System.Collections.Immutable.ImmutableList`1[Byte][System.Byte]:Sort(int,int,System.Collections.Generic.IComparer`1[Byte]):System.Collections.Immutable.ImmutableList`1[Byte]:this349 total methods with Code Size differences (347 improved, 2 regressed), 3 unchanged.

nathan-moore and ShreyasJejurkar reacted with thumbs up emoji
@ghostghost added the area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labelMay 25, 2021
@kunalspathak
Copy link
ContributorAuthor

@dotnet/jit-contrib

case INS_shl:
case INS_shr:
case INS_sar:
returnfalse;
Copy link
Member

@tannergoodingtannergoodingJun 2, 2021•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think this might require some careful considerations.

For the purposes of this PR, this is fine and is simply used as a "did the previous instruction set the flag and therefore atest can be skipped" check.

However, for future improvements we may consider more advanced analysis such as:

if (!IsFlagsModified(prevInstruction)){   // can depend on prevInstruction - 1 state being valid }

Then, this will become problematic. The latter happens in Clang/MSVC in various places and are one of the reasons thatmulx,rorx,sarx,shlx, andshrx are exposed on modern hardware.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Perhaps we can return some ternary state here representing "yes", "no", and "it depends"?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Today, we only track prevInstruction (emitLastIns). The way I read your comment, we will need to have access to last couple of instructions (if not more) to make sure the state is valid?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Right, it's not something I think we need to be concerned with "today", but rather something I could see being problematic in the future for certain optimizations.

The method implies boolean state but in practice its slightly more nuanced and one of the answers is "maybe", so it may end up confusing callers.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The method implies boolean state but in practice its slightly more nuanced and one of the answers is "maybe", so it may end up confusing callers.

Exactly - what are callers supposed to do with "maybe". So for now, I will leave it as-is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think that is contextual. In the case of this PR, you care "did the last instruction definitely set the flag I care about" and so you would treatmaybe asfalse.

In other cases, optimizations may care "did the last instruction definitely not set the flag I care about" and so they would treat it astrue.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Renamed the method toIsFlagsAlwaysModified.

@tannergooding
Copy link
Member

I think the changes overall look good and AFAICT are tracking the right state for the flags.

@kunalspathak
Copy link
ContributorAuthor

@BruceForstall or@AndyAyersMS, do you mind taking a look?

Copy link
Member

@AndyAyersMSAndyAyersMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Puzzled by a few things.

}

//------------------------------------------------------------------------
// IsFlagsAlwaysModified: check if the instruction always modifies the flags.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
//IsFlagsAlwaysModified: check if the instruction always modifies the flags.
//AreFlagsAlwaysModified: check if the instruction always modifies the flags.

I am a bit confused on what this is supposed to check. Does it mean: return true if there is at least one flag bit that is always modified by this instruction?

Seems odd we would ask that question instead of asking whether some specific set of flags is always modified.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

For shift instructions, if a shift-amount is0, it keeps the flags unmodified. For them, we shouldn't rely on last instruction, but the flags might have set by one of the previous instructions. As such, this method returnsfalse saying that these instructions (under certain circumstances) guaranteed to not modify flags and the caller shouldn't rely on last instruction to eliminatetest. For remaining instructions, they modifysome flags and can eliminatetest safely. I will add more explicit comment.

See my comment#53214 (comment) when I was not handling this case.

Copy link
Member

@AndyAyersMSAndyAyersMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks, LTGM.

@ghostghost locked asresolvedand limited conversation to collaboratorsJul 5, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@tannergoodingtannergoodingtannergooding left review comments

@AndyAyersMSAndyAyersMSAndyAyersMS approved these changes

+1 more reviewer

@nathan-moorenathan-moorenathan-moore left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kunalspathak@tannergooding@AndyAyersMS@nathan-moore
[8]ページ先頭
©2009-2025 Movatter.jp