JIT: Don't emit some unnecessary tests #38586
Conversation
Dotnet-GitSync-Bot
added
the
area-CodeGen-coreclr
|
@nathan-moore thanks for this PR. I actually would have expected a wider range of diffs -- eg an earlier prototype from @benaadams saw more cases. Would be good to understand what's different between that version and yours. @kunalspathak has created a tool in jit-utils to scan for patterns in asm, perhaps we could run it and see if there are more cases like this to catch? Can you also check for extended IGs as in Kunal's PR? Also can you double-check the examples from #32389? Here's a simple version you can try: using System;
using System.Runtime.CompilerServices;
using System.Runtime.Intrinsics;
class X
{
[MethodImpl(MethodImplOptions.NoInlining)]
static long Foo(long x)
{
long lengthToExamine = x - Vector256<byte>.Count;
if (lengthToExamine != 0)
{
return lengthToExamine;
}
else
{
return 0;
}
}
public static int Main()
{
return (int) Foo(0);
}
}cc @dotnet/jit-contrib |
|
No diffs in your example @AndyAyersMS , since the add turns into a lea. Ultimately, the lower diffs are due to the possibility of integer over/underflow, which makes the handling here have to be pretty conservative. If there's some sort of overflow analysis I can consume, we could probably get pretty close to a 4.5k diff that assumes no overflow, which is similar to what ben's prototype did. #6794 might also help recover some (most?) of these missed optimizations since this pr handles the common dec/inc case. |
|
With a clean jit-stress run I am going to merge it later today If there are no objections. |
ghost
locked as resolved and limited conversation to collaborators
Don't emit compares to 0 if the previous instruction has set the appropriate flags.
Fixes #10742
Diffs:
Total bytes of diff: -1069 (-0.003% of base) diff is an improvement. Top file improvements (bytes): -385 : System.Private.CoreLib.dasm (-0.012% of base) -87 : System.Private.Xml.dasm (-0.003% of base) -69 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (-0.002% of base) -68 : System.Memory.dasm (-0.079% of base) -64 : Microsoft.CodeAnalysis.CSharp.dasm (-0.003% of base) -30 : Microsoft.CSharp.dasm (-0.011% of base) -28 : System.Net.Http.dasm (-0.005% of base) -23 : System.Formats.Asn1.dasm (-0.036% of base) -21 : Microsoft.CodeAnalysis.dasm (-0.003% of base) -20 : System.Runtime.Numerics.dasm (-0.029% of base) -19 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.001% of base) -19 : System.Data.Common.dasm (-0.002% of base) -19 : System.Private.DataContractSerialization.dasm (-0.003% of base) -18 : System.Configuration.ConfigurationManager.dasm (-0.006% of base) -18 : System.Net.Mail.dasm (-0.011% of base) -18 : System.Reflection.Metadata.dasm (-0.006% of base) -13 : System.Security.Cryptography.Pkcs.dasm (-0.005% of base) -12 : ILCompiler.Reflection.ReadyToRun.dasm (-0.011% of base) -12 : System.Linq.Expressions.dasm (-0.000% of base) -12 : System.IO.Compression.dasm (-0.016% of base) 44 total files with Code Size differences (44 improved, 0 regressed), 220 unchanged. Top method improvements (bytes): -21 (-2.706% of base) : System.Private.Xml.dasm - BigNumber:Mul(byref):this -14 (-0.787% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - MemoryProcessMemInfoTraceData:ToXml(StringBuilder):StringBuilder:this -12 (-10.256% of base) : System.Private.CoreLib.dasm - CastHelpers:IsInstanceOfInterface(long,Object):Object -12 (-11.321% of base) : System.Private.CoreLib.dasm - CastHelpers:ChkCastInterface(long,Object):Object -11 (-2.075% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Binder:ShouldAddWinRTMembersForInterface(NamedTypeSymbol,NamedTypeSymbol,NamedTypeSymbol,NamedTypeSymbol,NamedTypeSymbol,NamedTypeSymbol,NamedTypeSymbol):bool -11 (-4.889% of base) : System.Private.CoreLib.dasm - SpanHelpers:SequenceEqual(byref,byref,long):bool -10 (-0.803% of base) : System.Security.Cryptography.Pkcs.dasm - Rfc3161TimestampTokenInfo:TryDecode(ReadOnlyMemory`1,bool,byref,byref,byref):bool -9 (-0.493% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceEventSymbol:CheckModifiersAndType(DiagnosticBag):this -9 (-0.700% of base) : System.Private.CoreLib.dasm - Span`1:Clear():this (18 methods) -9 (-1.339% of base) : System.Private.CoreLib.dasm - Span`1:Fill(long):this (3 methods) -9 (-0.723% of base) : System.Net.Http.dasm - HPackDecoder:Parse(ReadOnlySpan`1,byref,IHttpHeadersHandler):this -8 (-0.680% of base) : System.Private.DataContractSerialization.dasm - XmlConverter:TryParseDateTime(ref,int,int,byref):bool -7 (-1.781% of base) : System.Private.CoreLib.dasm - Buffer:Memmove(byref,byref,long) (3 methods) -7 (-2.071% of base) : System.Net.Mail.dasm - MailMessage:BuildDeliveryStatusNotificationString():String:this -7 (-1.505% of base) : System.Text.Json.dasm - JsonWriterHelper:TrimDateTimeOffset(Span`1,byref) -6 (-0.573% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourcePropertySymbol:CheckModifiers(Location,bool,DiagnosticBag):this -6 (-0.418% of base) : System.Private.CoreLib.dasm - DecCalc:DecAddSub(byref,byref,bool) -6 (-1.020% of base) : System.Private.CoreLib.dasm - Type:FilterAttributeImpl(MemberInfo,Object):bool -6 (-3.390% of base) : System.Private.CoreLib.dasm - Number:AccumulateDecimalDigitsIntoBigInteger(byref,int,int,byref) -6 (-0.772% of base) : System.Private.CoreLib.dasm - Number:TryNumberToDecimal(byref,byref):bool Top method improvements (percentages): -2 (-13.333% of base) : Microsoft.CSharp.dasm - TypeParameterSymbol:IsValueType():bool:this -2 (-13.333% of base) : Microsoft.CSharp.dasm - TypeParameterSymbol:IsReferenceType():bool:this -2 (-13.333% of base) : Microsoft.CSharp.dasm - TypeParameterSymbol:IsNonNullableValueType():bool:this -2 (-13.333% of base) : Microsoft.CSharp.dasm - TypeParameterSymbol:HasNewConstraint():bool:this -2 (-13.333% of base) : Microsoft.CSharp.dasm - TypeParameterSymbol:HasRefConstraint():bool:this -2 (-13.333% of base) : Microsoft.CSharp.dasm - TypeParameterSymbol:HasValConstraint():bool:this -12 (-11.321% of base) : System.Private.CoreLib.dasm - CastHelpers:ChkCastInterface(long,Object):Object -2 (-10.526% of base) : Microsoft.CSharp.dasm - TypeParameterType:get_IsValueType():bool:this -2 (-10.526% of base) : Microsoft.CSharp.dasm - TypeParameterType:get_IsReferenceType():bool:this -2 (-10.526% of base) : Microsoft.CSharp.dasm - TypeParameterType:get_IsNonNullableValueType():bool:this -2 (-10.526% of base) : Microsoft.CSharp.dasm - TypeParameterType:get_HasNewConstraint():bool:this -2 (-10.526% of base) : Microsoft.CSharp.dasm - TypeParameterType:get_HasRefConstraint():bool:this -2 (-10.526% of base) : Microsoft.CSharp.dasm - TypeParameterType:get_HasValConstraint():bool:this -2 (-10.526% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - TypeFlagsHelpers:GetArrayRank(int):int -12 (-10.256% of base) : System.Private.CoreLib.dasm - CastHelpers:IsInstanceOfInterface(long,Object):Object -2 (-7.692% of base) : Microsoft.CodeAnalysis.dasm - GenericParamRowComparer:Compare(GenericParamRow,GenericParamRow):int:this -2 (-6.897% of base) : System.Private.CoreLib.dasm - ASCIIUtility:CountNumberOfLeadingAsciiBytesFromUInt32WithSomeNonAsciiData(int):int -2 (-6.452% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - SourcePropertyAccessorSymbol:get_DeclaredAccessibility():int:this -5 (-6.250% of base) : System.Private.CoreLib.dasm - BigInteger:Compare(byref,byref):int -2 (-5.714% of base) : System.Private.CoreLib.dasm - StringBuilder:Append(StringBuilder):StringBuilder:this 361 total methods with Code Size differences (361 improved, 0 regressed), 181257 unchanged.