emit test for bounds checks against a 0 index #42295
Conversation
Dotnet-GitSync-Bot
added
the
area-CodeGen-coreclr
|
Nice! |
|
(oops forgot my approves are "green" in this repo - treat it is a "like" 🙂) |
sandreenko
left a comment
sandreenko
left a comment
There was a problem hiding this comment.
LGTM, thank you, do you know if we can do something similar for arm64?
|
cc @dotnet/jit-contrib |
|
Thanks @nathan-moore for your contribution. |
static int Test(int[] arr) => arr[0];Current codegen: G_M50965_IG01:
A9BF7BFD stp fp, lr, [sp,#-16]!
910003FD mov fp, sp
;; bbWeight=1 PerfScore 1.50
G_M50965_IG02:
B9400801 ldr w1, [x0,#8]
7100003F cmp w1, #0
54000089 bls G_M50965_IG04
B9401000 ldr w0, [x0,#16]
;; bbWeight=1 PerfScore 7.50
G_M50965_IG03:
A8C17BFD ldp fp, lr, [sp],#16
D65F03C0 ret lr
;; bbWeight=1 PerfScore 2.00
G_M50965_IG04:
97FF2D9A bl CORINFO_HELP_RNGCHKFAIL
D4200000 brk #0
;; bbWeight=0 PerfScore 0.00this cmp w1, #0
bls G_M50965_IG04can be optimized to: cbz w1, G_M50965_IG04 ;; or CBNZbranch if (not)zero. |
|
@sandreenko @EgorBo I looked into what it will take to emit cbz here for arm. There isn't a way to emit it with genJumpToThrowHlpBlk, which is what everything currently uses. You can add a way, but since cbz is internally represented very differently from a normal jump, I don't see a good way of not duplicating the logic, which is what I did here: nathan-moore@8500e53. Does that seem like the right approach? Also, I checked and there's already a peephole for this in the general case. |
|
Thank you @nathan-moore and @EgorBo for the analysis. I think it is valuable to add this optimization for arm64 if it produces similar diffs there. However, it could be done in a separate PR and this one can be merged now. Thanks @nathan-moore for this optimization. |
|
Opened a follow-up for arm64: #42514. |
ghost
locked as resolved and limited conversation to collaborators
5 participants
Since array length is always non-negative, when we have a bounds check against a constant 0 index, we can emit a slightly cheaper/smaller test.
Found 156 files with textual diffs. Summary of Code Size diffs: (Lower is better) Total bytes of diff: -3809 (-0.012% of base) diff is an improvement. Top file improvements (bytes): -891 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (-0.027% of base) -597 : System.Private.CoreLib.dasm (-0.018% of base) -254 : System.Private.Xml.dasm (-0.008% of base) -121 : System.Data.Common.dasm (-0.011% of base) -101 : Microsoft.VisualBasic.Core.dasm (-0.024% of base) -77 : System.Net.Http.dasm (-0.013% of base) -68 : Microsoft.CodeAnalysis.dasm (-0.009% of base) -67 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.003% of base) -61 : System.Linq.Expressions.dasm (-0.002% of base) -57 : System.Runtime.Numerics.dasm (-0.083% of base) -51 : Microsoft.CodeAnalysis.CSharp.dasm (-0.002% of base) -51 : System.Private.DataContractSerialization.dasm (-0.007% of base) -50 : System.Drawing.Common.dasm (-0.018% of base) -44 : Newtonsoft.Json.dasm (-0.007% of base) -40 : System.CodeDom.dasm (-0.024% of base) -38 : System.Data.Odbc.dasm (-0.021% of base) -35 : FSharp.Core.dasm (-0.004% of base) -35 : System.Text.Json.dasm (-0.012% of base) -34 : System.Net.Primitives.dasm (-0.052% of base) -34 : System.ComponentModel.TypeConverter.dasm (-0.015% of base) 156 total files with Code Size differences (156 improved, 0 regressed), 111 unchanged. Top method improvements (bytes): -38 (-1.332% of base) : System.Private.CoreLib.dasm - ArraySortHelper`2:HeapSort(Span`1,Span`1,IComparer`1) (11 methods) -30 (-1.162% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`2:HeapSort(Span`1,Span`1) (11 methods) -22 (-0.285% of base) : System.Private.CoreLib.dasm - Vector128`1:ToString():String:this (11 methods) -22 (-0.267% of base) : System.Private.CoreLib.dasm - Vector256`1:ToString():String:this (11 methods) -22 (-0.277% of base) : System.Private.CoreLib.dasm - Vector64`1:ToString():String:this (11 methods) -15 (-0.169% of base) : System.Private.Xml.dasm - XsltLoader:.ctor():this -14 (-0.536% of base) : System.Private.CoreLib.dasm - ArraySortHelper`1:HeapSort(Span`1,Comparison`1) (14 methods) -14 (-0.491% of base) : System.Net.Http.dasm - KnownHeaders:GetCandidate(StringAccessor):KnownHeader -13 (-0.569% of base) : System.Private.CoreLib.dasm - GenericArraySortHelper`1:HeapSort(Span`1) (13 methods) -12 (-0.259% of base) : System.Data.Common.dasm - FunctionNode:EvalFunction(int,ref,DataRow,int):Object:this -11 (-0.197% of base) : System.CodeDom.dasm - VBCodeGenerator:.cctor() -8 (-0.726% of base) : CommandLine.dasm - <>c:<get_FormatError>b__11_0(Error):String:this -7 (-2.160% of base) : Microsoft.CodeAnalysis.CSharp.dasm - CSharpSyntaxNode:CreateList(IEnumerable`1,bool):GreenNode:this -7 (-3.182% of base) : Newtonsoft.Json.Bson.dasm - BsonDataReader:BytesInSequence(ubyte):int:this -7 (-3.365% of base) : Newtonsoft.Json.Bson.dasm - BsonDataReader:.cctor() -7 (-3.125% of base) : Newtonsoft.Json.dasm - BsonReader:.cctor() -7 (-0.421% of base) : System.Data.Common.dasm - SqlDecimal:MpDiv(ReadOnlySpan`1,int,Span`1,int,Span`1,byref,Span`1,byref) -6 (-1.575% of base) : System.Private.CoreLib.dasm - Number:FormatExponent(byref,NumberFormatInfo,int,ushort,int,bool) -6 (-0.311% of base) : System.Private.CoreLib.dasm - Number:.cctor() -6 (-0.286% of base) : System.Private.CoreLib.dasm - CalendarData:CreateInvariant():CalendarData Top method improvements (percentages): -7 (-3.365% of base) : Newtonsoft.Json.Bson.dasm - BsonDataReader:.cctor() -7 (-3.182% of base) : Newtonsoft.Json.Bson.dasm - BsonDataReader:BytesInSequence(ubyte):int:this -1 (-3.125% of base) : System.Collections.Immutable.dasm - ImmutableArrayExtensions:FirstOrDefault(ImmutableArray`1):__Canon -7 (-3.125% of base) : Newtonsoft.Json.dasm - BsonReader:.cctor() -1 (-2.703% of base) : Microsoft.VisualBasic.Core.dasm - CharType:FromString(String):ushort -1 (-2.703% of base) : Microsoft.VisualBasic.Core.dasm - Conversions:ToChar(String):ushort -1 (-2.564% of base) : System.Private.Xml.dasm - Compiler:IsPhantomNamespace(String):bool:this -1 (-2.564% of base) : xunit.execution.dotnet.dasm - Pack:UInt16_To_LE(ushort,ref) -1 (-2.500% of base) : System.Private.CoreLib.dasm - Statics:ShouldOverrideFieldName(String):bool -1 (-2.500% of base) : Microsoft.VisualBasic.Core.dasm - FileSystem:ChDrive(String) -1 (-2.500% of base) : System.Private.Xml.dasm - OutputScopeManager:Reset():this -1 (-2.500% of base) : xunit.execution.dotnet.dasm - Pack:UInt16_To_BE(ushort,ref) -1 (-2.439% of base) : System.Private.Xml.dasm - BigInteger:InitFromDigits(int,int,int):this -5 (-2.427% of base) : Microsoft.CodeAnalysis.dasm - UnionCollection`1:Create(ImmutableArray`1,Func`2):ICollection`1 -1 (-2.381% of base) : Microsoft.CodeAnalysis.CSharp.dasm - CodeGenerator:IsMultidimensionalInitializer(ImmutableArray`1):bool -1 (-2.381% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - CodeGenerator:IsMultidimensionalInitializer(ImmutableArray`1):bool:this -1 (-2.381% of base) : xunit.execution.dotnet.dasm - Pack:BE_To_UInt16(ref):ushort -1 (-2.381% of base) : xunit.execution.dotnet.dasm - Pack:LE_To_UInt16(ref):ushort -1 (-2.326% of base) : Newtonsoft.Json.Bson.dasm - StringUtils:StartsWith(String,ushort):bool -1 (-2.326% of base) : Newtonsoft.Json.dasm - StringUtils:StartsWith(String,ushort):bool 3127 total methods with Code Size differences (3127 improved, 0 regressed), 188060 unchanged.