RyuJIT: Optimize "x<0" to "x>>31"

[EgorBo]

I am not a codegen expert and just trying to learn this stuff through mistakes and your feedback. I wonder if this is a correct way to optimize y=x<0 to y=x>>31 (when y is not a jump condition - in that case test is used)

static bool Test_sbyte(sbyte a) => a < 0; static bool Test_short(short a) => a < 0; static bool Test_int(int a) => a < 0; static bool Test_long(long a) => a < 0;

Before

; Method Test_sbyte(byte):bool  480FBEC1 movsx rax, cl  85C0 test eax, eax  0F9CC0 setl al  0FB6C0 movzx rax, al  C3 ret  ; Total bytes of code: 13 ; Method Test_short(short):bool  480FBFC1 movsx rax, cx  85C0 test eax, eax  0F9CC0 setl al  0FB6C0 movzx rax, al  C3 ret  ; Total bytes of code: 13 ; Method Test_int(int):bool  85C9 test ecx, ecx  0F9CC0 setl al  0FB6C0 movzx rax, al  C3 ret  ; Total bytes of code: 9 ; Method Test_long(long):bool  4885C9 test rcx, rcx  0F9CC0 setl al  0FB6C0 movzx rax, al  C3 ret  ; Total bytes of code: 10

After

; Method Test_sbyte(byte):bool  480FBEC1 movsx rax, cl  C1E81F shr eax, 31  C3 ret  ; Total bytes of code: 8 ; Method Test_short(short):bool  480FBFC1 movsx rax, cx  C1E81F shr eax, 31  C3 ret  ; Total bytes of code: 8 ; Method Test_int(int):bool  488BC1 mov rax, rcx  C1E81F shr eax, 31  C3 ret  ; Total bytes of code: 7 ; Method Test_long(long):bool  488BC1 mov rax, rcx  48C1E83F shr rax, 63  C3 ret  ; Total bytes of code: 8

jit-diff (-f -pmi)

C:\prj>jit-diff diff --output C:\prj\jitdiffs -f --core_root C:\prj\runtime-1\artifacts\tests\coreclr\Windows_NT.x64.Release\Tests\Core_Root --base C:\prj\runtime-1\artifacts\bin\coreclr\Windows_NT.x64.Checked_base --diff C:\prj\runtime-1\artifacts\bin\coreclr\Windows_NT.x64.Checked --pmi Beginning PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies / Finished 267/267 Base 267/267 Diff [345.4 sec] Completed PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies in 345.57s Diffs (if any) can be viewed by comparing: C:\prj\jitdiffs\dasmset_4\base C:\prj\jitdiffs\dasmset_4\diff Analyzing CodeSize diffs... Found 76 files with textual diffs. PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for default jit Summary of Code Size diffs: (Lower is better) Total bytes of diff: -2451 (-0.00% of base) diff is an improvement. Top file improvements (bytes): -282 : System.Private.CoreLib.dasm (-0.01% of base) -244 : Microsoft.CodeAnalysis.dasm (-0.01% of base) -224 : Microsoft.CodeAnalysis.CSharp.dasm (-0.01% of base) -208 : System.Collections.Immutable.dasm (-0.02% of base) -179 : System.Collections.dasm (-0.04% of base) -173 : System.Runtime.Numerics.dasm (-0.24% of base) -112 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.00% of base) -96 : Newtonsoft.Json.dasm (-0.01% of base) -96 : System.Linq.dasm (-0.01% of base) -92 : System.Data.Common.dasm (-0.01% of base) -59 : FSharp.Core.dasm (-0.00% of base) -45 : System.Security.Cryptography.Primitives.dasm (-0.13% of base) -42 : System.Memory.dasm (-0.02% of base) -33 : System.Reflection.MetadataLoadContext.dasm (-0.02% of base) -32 : System.Linq.Parallel.dasm (-0.00% of base) -26 : System.Configuration.ConfigurationManager.dasm (-0.01% of base) -26 : System.Linq.Expressions.dasm (-0.00% of base) -24 : System.Data.OleDb.dasm (-0.01% of base) -23 : System.IO.Compression.dasm (-0.03% of base) -23 : xunit.runner.utility.netcoreapp10.dasm (-0.01% of base) 75 total files with Code Size differences (75 improved, 0 regressed), 192 unchanged. Top method improvements (bytes): -45 (-1.71% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:CheckConstantBounds(byte,Decimal):bool -35 (-0.85% of base) : System.Security.Cryptography.Primitives.dasm - AsymmetricAlgorithm:ExportArray(ReadOnlySpan`1,PbeParameters,TryExportPbe`1):ref (7 methods) -30 (-17.86% of base) : Microsoft.CodeAnalysis.dasm - <>c__DisplayClass27_0:<GetSyntaxNodesToAnalyze>b__0(SyntaxNode):bool:this (6 methods) -30 (-4.71% of base) : System.Runtime.Numerics.dasm - BigInteger:op_Multiply(BigInteger,BigInteger):BigInteger -28 (-1.48% of base) : Newtonsoft.Json.dasm - JsonTextWriter:DoWriteValueAsync(Nullable`1,CancellationToken):Task:this (15 methods) -26 (-6.68% of base) : Microsoft.CodeAnalysis.dasm - ConstantValue:get_IsNegativeNumeric():bool:this -23 (-1.93% of base) : System.IO.Compression.dasm - Zip64ExtraField:TryGetZip64BlockFromGenericExtraField(ZipGenericExtraField,bool,bool,bool,bool,byref):bool -21 (-20.00% of base) : System.Collections.Concurrent.dasm - WorkStealingQueue:get_IsEmpty():bool:this (7 methods) -21 (-2.95% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.Contains(Object):bool:this (7 methods) -21 (-6.98% of base) : System.Collections.dasm - SortedList`2:ContainsValue(long):bool:this (7 methods) -21 (-8.11% of base) : System.Collections.dasm - ValueList:Contains(long):bool:this (7 methods) -21 (-1.75% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:System.Collections.IList.Contains(Object):bool:this (7 methods) -21 (-2.56% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IList.Contains(Object):bool:this (7 methods) -21 (-0.36% of base) : System.Collections.Immutable.dasm - Enumerator:.ctor(Node,Builder,int,int,bool):this (7 methods) -21 (-1.73% of base) : System.Linq.dasm - EnumerableHelpers:TryGetCount(IEnumerable`1,byref):bool (7 methods) -21 (-6.98% of base) : System.Linq.dasm - Grouping`2:System.Collections.Generic.ICollection<TElement>.Contains(long):bool:this (7 methods) -21 (-0.38% of base) : System.Linq.Parallel.dasm - NullableDecimalMinMaxAggregationOperatorEnumerator`1:MoveNextCore(byref):bool:this (7 methods) -21 (-1.42% of base) : System.Private.CoreLib.dasm - HashSet`1:IsSubsetOf(IEnumerable`1):bool:this (7 methods) -18 (-0.19% of base) : Microsoft.CodeAnalysis.dasm - AnalyzerExecutor:ExecuteCodeBlockActionsCore(IEnumerable`1,IEnumerable`1,IEnumerable`1,DiagnosticAnalyzer,SyntaxNode,ISymbol,ImmutableArray`1,SemanticModel,Func`2,CodeBlockAnalyzerStateData):this (6 methods) -18 (-0.69% of base) : Microsoft.CodeAnalysis.dasm - AnalyzerExecutor:ExecuteSyntaxNodeActions(IEnumerable`1,IDictionary`2,SemanticModel,Func`2,Action`1,SyntaxNodeAnalyzerStateData):this (6 methods) Top method improvements (percentages): -6 (-33.33% of base) : System.Private.CoreLib.dasm - Double:IsNegative(double):bool -5 (-31.25% of base) : System.Private.CoreLib.dasm - Half:IsNegative(Half):bool -5 (-31.25% of base) : System.Private.CoreLib.dasm - Single:IsNegative(float):bool -5 (-22.73% of base) : System.Private.CoreLib.dasm - Decimal:op_LessThan(Decimal,Decimal):bool -5 (-22.73% of base) : System.Private.Xml.Linq.dasm - XNode:IsBefore(XNode):bool:this -5 (-22.73% of base) : System.Runtime.Numerics.dasm - BigInteger:op_LessThan(BigInteger,BigInteger):bool -10 (-22.73% of base) : System.Runtime.Numerics.dasm - BigInteger:op_LessThan(BigInteger,long):bool (2 methods) -21 (-20.00% of base) : System.Collections.Concurrent.dasm - WorkStealingQueue:get_IsEmpty():bool:this (7 methods) -5 (-17.86% of base) : Microsoft.CodeAnalysis.CSharp.dasm - <>c__DisplayClass153_0:<FilterOverriddenOrHiddenMethods>b__0(MethodSymbol):bool:this -30 (-17.86% of base) : Microsoft.CodeAnalysis.dasm - <>c__DisplayClass27_0:<GetSyntaxNodesToAnalyze>b__0(SyntaxNode):bool:this (6 methods) -5 (-17.86% of base) : xunit.execution.dotnet.dasm - XunitTestClassRunner:<CreateClassFixtureAsync>b__11_0(IAsyncLifetime):bool:this -10 (-16.13% of base) : System.Runtime.Numerics.dasm - BigInteger:op_GreaterThan(long,BigInteger):bool (2 methods) -5 (-15.62% of base) : System.ComponentModel.Composition.dasm - CatalogChangeProxy:<GetExports>b__5_0(Tuple`2):bool:this -3 (-13.64% of base) : Microsoft.Extensions.Primitives.dasm - StringValues:System.Collections.Generic.ICollection<System.String>.Contains(String):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(__Canon):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(int):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(Vector`1):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(long):bool:this -3 (-13.64% of base) : System.ComponentModel.Composition.dasm - WeakReferenceCollection`1:Contains(__Canon):bool:this -3 (-13.64% of base) : System.Data.Common.dasm - ConstraintCollection:Contains(String):bool:this 666 total methods with Code Size differences (666 improved, 0 regressed), 258004 unchanged. Completed analysis in 34.73s 

[a-tk-by]

Since .NET uses eax register in x64 mode to return boolean you don't need to extend cl/cx register to rax. eax is enough, so there is no need for 48 prefix in opcode.

[BruceForstall]

@dotnet/jit-contrib

[EgorBo]

New jit-diff (there are no regressions):

C:\prj>jit-diff diff --output C:\prj\jitdiffs -f --core_root C:\prj\runtime-1\artifacts\tests\coreclr\Windows_NT.x64.Release\Tests\Core_Root --base C:\prj\runtime-1\artifacts\bin\coreclr\Windows_NT.x64.Checked_base --diff C:\prj\runtime-1\artifacts\bin\coreclr\Windows_NT.x64.Checked --pmi Beginning PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies / Finished 267/267 Base 267/267 Diff [345.4 sec] Completed PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies in 345.57s Diffs (if any) can be viewed by comparing: C:\prj\jitdiffs\dasmset_4\base C:\prj\jitdiffs\dasmset_4\diff Analyzing CodeSize diffs... Found 76 files with textual diffs. PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for default jit Summary of Code Size diffs: (Lower is better) Total bytes of diff: -2451 (-0.00% of base) diff is an improvement. Top file improvements (bytes): -282 : System.Private.CoreLib.dasm (-0.01% of base) -244 : Microsoft.CodeAnalysis.dasm (-0.01% of base) -224 : Microsoft.CodeAnalysis.CSharp.dasm (-0.01% of base) -208 : System.Collections.Immutable.dasm (-0.02% of base) -179 : System.Collections.dasm (-0.04% of base) -173 : System.Runtime.Numerics.dasm (-0.24% of base) -112 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.00% of base) -96 : Newtonsoft.Json.dasm (-0.01% of base) -96 : System.Linq.dasm (-0.01% of base) -92 : System.Data.Common.dasm (-0.01% of base) -59 : FSharp.Core.dasm (-0.00% of base) -45 : System.Security.Cryptography.Primitives.dasm (-0.13% of base) -42 : System.Memory.dasm (-0.02% of base) -33 : System.Reflection.MetadataLoadContext.dasm (-0.02% of base) -32 : System.Linq.Parallel.dasm (-0.00% of base) -26 : System.Configuration.ConfigurationManager.dasm (-0.01% of base) -26 : System.Linq.Expressions.dasm (-0.00% of base) -24 : System.Data.OleDb.dasm (-0.01% of base) -23 : System.IO.Compression.dasm (-0.03% of base) -23 : xunit.runner.utility.netcoreapp10.dasm (-0.01% of base) 75 total files with Code Size differences (75 improved, 0 regressed), 192 unchanged. Top method improvements (bytes): -45 (-1.71% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:CheckConstantBounds(byte,Decimal):bool -35 (-0.85% of base) : System.Security.Cryptography.Primitives.dasm - AsymmetricAlgorithm:ExportArray(ReadOnlySpan`1,PbeParameters,TryExportPbe`1):ref (7 methods) -30 (-17.86% of base) : Microsoft.CodeAnalysis.dasm - <>c__DisplayClass27_0:<GetSyntaxNodesToAnalyze>b__0(SyntaxNode):bool:this (6 methods) -30 (-4.71% of base) : System.Runtime.Numerics.dasm - BigInteger:op_Multiply(BigInteger,BigInteger):BigInteger -28 (-1.48% of base) : Newtonsoft.Json.dasm - JsonTextWriter:DoWriteValueAsync(Nullable`1,CancellationToken):Task:this (15 methods) -26 (-6.68% of base) : Microsoft.CodeAnalysis.dasm - ConstantValue:get_IsNegativeNumeric():bool:this -23 (-1.93% of base) : System.IO.Compression.dasm - Zip64ExtraField:TryGetZip64BlockFromGenericExtraField(ZipGenericExtraField,bool,bool,bool,bool,byref):bool -21 (-20.00% of base) : System.Collections.Concurrent.dasm - WorkStealingQueue:get_IsEmpty():bool:this (7 methods) -21 (-2.95% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.Contains(Object):bool:this (7 methods) -21 (-6.98% of base) : System.Collections.dasm - SortedList`2:ContainsValue(long):bool:this (7 methods) -21 (-8.11% of base) : System.Collections.dasm - ValueList:Contains(long):bool:this (7 methods) -21 (-1.75% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:System.Collections.IList.Contains(Object):bool:this (7 methods) -21 (-2.56% of base) : System.Collections.Immutable.dasm - Builder:System.Collections.IList.Contains(Object):bool:this (7 methods) -21 (-0.36% of base) : System.Collections.Immutable.dasm - Enumerator:.ctor(Node,Builder,int,int,bool):this (7 methods) -21 (-1.73% of base) : System.Linq.dasm - EnumerableHelpers:TryGetCount(IEnumerable`1,byref):bool (7 methods) -21 (-6.98% of base) : System.Linq.dasm - Grouping`2:System.Collections.Generic.ICollection<TElement>.Contains(long):bool:this (7 methods) -21 (-0.38% of base) : System.Linq.Parallel.dasm - NullableDecimalMinMaxAggregationOperatorEnumerator`1:MoveNextCore(byref):bool:this (7 methods) -21 (-1.42% of base) : System.Private.CoreLib.dasm - HashSet`1:IsSubsetOf(IEnumerable`1):bool:this (7 methods) -18 (-0.19% of base) : Microsoft.CodeAnalysis.dasm - AnalyzerExecutor:ExecuteCodeBlockActionsCore(IEnumerable`1,IEnumerable`1,IEnumerable`1,DiagnosticAnalyzer,SyntaxNode,ISymbol,ImmutableArray`1,SemanticModel,Func`2,CodeBlockAnalyzerStateData):this (6 methods) -18 (-0.69% of base) : Microsoft.CodeAnalysis.dasm - AnalyzerExecutor:ExecuteSyntaxNodeActions(IEnumerable`1,IDictionary`2,SemanticModel,Func`2,Action`1,SyntaxNodeAnalyzerStateData):this (6 methods) Top method improvements (percentages): -6 (-33.33% of base) : System.Private.CoreLib.dasm - Double:IsNegative(double):bool -5 (-31.25% of base) : System.Private.CoreLib.dasm - Half:IsNegative(Half):bool -5 (-31.25% of base) : System.Private.CoreLib.dasm - Single:IsNegative(float):bool -5 (-22.73% of base) : System.Private.CoreLib.dasm - Decimal:op_LessThan(Decimal,Decimal):bool -5 (-22.73% of base) : System.Private.Xml.Linq.dasm - XNode:IsBefore(XNode):bool:this -5 (-22.73% of base) : System.Runtime.Numerics.dasm - BigInteger:op_LessThan(BigInteger,BigInteger):bool -10 (-22.73% of base) : System.Runtime.Numerics.dasm - BigInteger:op_LessThan(BigInteger,long):bool (2 methods) -21 (-20.00% of base) : System.Collections.Concurrent.dasm - WorkStealingQueue:get_IsEmpty():bool:this (7 methods) -5 (-17.86% of base) : Microsoft.CodeAnalysis.CSharp.dasm - <>c__DisplayClass153_0:<FilterOverriddenOrHiddenMethods>b__0(MethodSymbol):bool:this -30 (-17.86% of base) : Microsoft.CodeAnalysis.dasm - <>c__DisplayClass27_0:<GetSyntaxNodesToAnalyze>b__0(SyntaxNode):bool:this (6 methods) -5 (-17.86% of base) : xunit.execution.dotnet.dasm - XunitTestClassRunner:<CreateClassFixtureAsync>b__11_0(IAsyncLifetime):bool:this -10 (-16.13% of base) : System.Runtime.Numerics.dasm - BigInteger:op_GreaterThan(long,BigInteger):bool (2 methods) -5 (-15.62% of base) : System.ComponentModel.Composition.dasm - CatalogChangeProxy:<GetExports>b__5_0(Tuple`2):bool:this -3 (-13.64% of base) : Microsoft.Extensions.Primitives.dasm - StringValues:System.Collections.Generic.ICollection<System.String>.Contains(String):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(__Canon):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(int):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(Vector`1):bool:this -3 (-13.64% of base) : System.Collections.dasm - SortedList`2:ContainsKey(long):bool:this -3 (-13.64% of base) : System.ComponentModel.Composition.dasm - WeakReferenceCollection`1:Contains(__Canon):bool:this -3 (-13.64% of base) : System.Data.Common.dasm - ConstraintCollection:Contains(String):bool:this 666 total methods with Code Size differences (666 improved, 0 regressed), 258004 unchanged. Completed analysis in 34.73s 

[EgorBo]

@CarolEidt should be better now: x86 support is checked, x>=0 case is added, PR is green.

[CarolEidt]

LGTM - thanks @EgorBo !

[sandreenko]

I suspect this change has broken such CMP in minopts:

N004 ( 1, 1) [000002] ------------ t2 = CNS_INT int -1 REG rax N006 ( 1, 1) [000001] -c---------- t1 = CNS_INT int 0 REG NA /--* t2 int +--* t1 int N008 ( 6, 3) [000003] N--------U-- t3 = * GE int REG rax // note that it is unsigned, the expected value is true 

the generated code is:

IN0001: mov eax, -1 // 0xFFFF IN0002: not eax // 0x0000 IN0003: shr eax, 31 // 0x0000 where we expect 0x0001 

This was found during library stress runs: #42719

I have attached a minimal repo
Runtime_42719.txt

@EgorBo could you please take a look?

[EgorBo]

Sure! I'm taking a look now.