Use GT_NULLCHECK for unconsumed indirections. by erozenfeld · Pull Request #32641 · dotnet/runtime

erozenfeld · 2020-02-21T06:30:01Z

All unconsumed indirections are now GT_NULLCHECKs, which
allows optFoldNullChecks to remove more redundant null checks.

All unconsumed indirections are now GT_NULLCHECKs, which allows optFoldNullChecks to remove more redundant null checks.

erozenfeld · 2020-02-21T06:31:21Z

Framework diffs (x64 pmi):

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: -33587 (-0.07% of base)
    diff is an improvement.
Top file regressions (bytes):
          58 : System.Diagnostics.Process.dasm (0.06% of base)
Top file improvements (bytes):
       -8254 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.15% of base)
       -7309 : Microsoft.CodeAnalysis.CSharp.dasm (-0.17% of base)
       -2777 : Microsoft.CodeAnalysis.dasm (-0.16% of base)
       -2193 : System.Private.Xml.dasm (-0.06% of base)
       -1138 : System.Private.CoreLib.dasm (-0.02% of base)
       -1012 : Microsoft.CSharp.dasm (-0.33% of base)
        -895 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (-0.03% of base)
        -794 : System.Collections.Immutable.dasm (-0.07% of base)
        -729 : System.Data.Common.dasm (-0.05% of base)
        -713 : System.Data.SqlClient.dasm (-0.08% of base)
        -612 : System.Reflection.Metadata.dasm (-0.14% of base)
        -466 : System.Net.Http.dasm (-0.07% of base)
        -455 : System.Private.DataContractSerialization.dasm (-0.06% of base)
        -438 : System.Collections.dasm (-0.08% of base)
        -418 : System.Configuration.ConfigurationManager.dasm (-0.12% of base)
        -376 : System.Collections.Concurrent.dasm (-0.11% of base)
        -323 : System.Threading.Tasks.Dataflow.dasm (-0.04% of base)
        -292 : System.DirectoryServices.AccountManagement.dasm (-0.08% of base)
        -239 : System.Management.dasm (-0.06% of base)
        -225 : System.Reflection.MetadataLoadContext.dasm (-0.12% of base)
123 total files with Code Size differences (122 improved, 1 regressed), 110 unchanged.
Top method regressions (bytes):
          94 ( 3.83% of base) : System.Diagnostics.Process.dasm - NtProcessManager:GetProcessInfos(PerformanceCounterLib,int,int,ReadOnlySpan`1):ref
           3 ( 0.12% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceNamedTypeSymbol:MakeDeclaredBases(ConsList`1,DiagnosticBag):Tuple`2:this
           2 ( 0.08% of base) : System.Configuration.ConfigurationManager.dasm - BaseConfigurationRecord:InitConfigFromFile():this
           2 ( 3.17% of base) : System.Data.SqlClient.dasm - TdsParserStateObject:IncrementAndObtainOpenResultCount(SqlInternalTransaction):int:this
           2 ( 0.97% of base) : System.Private.Xml.dasm - XmlQueryType:CreateTypeCodeDerivation():BitMatrix
           2 ( 0.07% of base) : System.Private.Xml.dasm - DecimalFormatter:.ctor(String,DecimalFormat):this
           1 ( 0.13% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeTablesHeader(BlobBuilder,MetadataSizes):this
           1 ( 0.07% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeMetadataHeader(BlobBuilder,MetadataSizes):this
           1 ( 0.07% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeMetadataTables(BlobBuilder,MetadataSizes,int,int):this
           1 ( 0.05% of base) : System.Collections.Concurrent.dasm - ConcurrentDictionary`2:System.Collections.IDictionary.set_Item(Object,Object):this (7 methods)
Top method improvements (bytes):
        -274 (-0.67% of base) : Microsoft.CodeAnalysis.dasm - ArrayBuilder`1:ToDictionary(Func`2,IEqualityComparer`1):Dictionary`2:this (49 methods)
        -182 (-0.53% of base) : System.Data.Common.dasm - SortExpressionBuilder`1:CloneCast():SortExpressionBuilder`1:this (49 methods)
        -139 (-4.90% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - OverloadResolution:DetermineMostSpecificNarrowingConversion(TypeSymbol,TypeSymbol,ArrayBuilder`1,ArrayBuilder`1,byref,byref,bool,byref):bool
        -138 (-2.18% of base) : Microsoft.CodeAnalysis.dasm - ArrayBuilderExtensions:SelectAsArray(ArrayBuilder`1,Func`2):ImmutableArray`1 (7 methods)
        -129 (-7.01% of base) : Microsoft.CodeAnalysis.CSharp.dasm - OverloadResolution:BinaryOperatorOverloadResolution(BoundExpression,BoundExpression,BinaryOperatorOverloadResolutionResult,byref):this
        -125 (-0.59% of base) : Microsoft.CodeAnalysis.dasm - ArrayBuilder`1:SelectDistinct(Func`2):ImmutableArray`1:this (49 methods)
        -102 (-7.09% of base) : Microsoft.CodeAnalysis.CSharp.dasm - OverloadResolution:UnaryOperatorOverloadResolution(BoundExpression,UnaryOperatorOverloadResolutionResult,byref):this
         -78 (-4.29% of base) : System.Private.Xml.dasm - XsltLoader:LoadRealStylesheet():this
         -77 (-5.58% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SyntaxFactory:NodesAreCorrectType(SyntaxNodeOrTokenList):bool (7 methods)
         -77 (-5.67% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - SyntaxFactory:NodesAreCorrectType(SyntaxNodeOrTokenList):bool (7 methods)
         -77 (-1.43% of base) : System.Collections.Concurrent.dasm - BlockingCollection`1:TryTakeWithNoTimeValidation(byref,int,CancellationToken,CancellationTokenSource):bool:this (7 methods)
         -65 (-1.36% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeTypeReference(ITypeReference,BlobBuilder,bool,bool):this
         -64 (-3.61% of base) : Microsoft.CodeAnalysis.dasm - ArrayBuilder`1:RemoveDuplicates():this (7 methods)
         -58 (-1.92% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Parser:ParseTerm(bool,bool):ExpressionSyntax:this
         -56 (-1.95% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Parser:ParseInterpolatedStringInterpolation():InterpolationSyntax:this
         -56 (-1.29% of base) : System.Private.CoreLib.dasm - TextInfo:ChangeCaseCommon(String):String:this (8 methods)
         -53 (-0.80% of base) : Microsoft.CodeAnalysis.CSharp.dasm - CSharpDeclarationComputer:ComputeDeclarations(SemanticModel,SyntaxNode,Func`3,bool,List`1,Nullable`1,CancellationToken)
         -51 (-0.88% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - VisualBasicDeclarationComputer:ComputeDeclarationsCore(SemanticModel,SyntaxNode,Func`3,bool,List`1,Nullable`1,CancellationToken)
         -50 (-4.05% of base) : System.Private.Uri.dasm - Uri:PrivateParseMinimal():int:this
         -49 (-0.34% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - MethodToClassRewriter`1:RewriteBlock(BoundBlock,ArrayBuilder`1,ArrayBuilder`1):BoundBlock:this (7 methods)
Top method regressions (percentages):
          94 ( 3.83% of base) : System.Diagnostics.Process.dasm - NtProcessManager:GetProcessInfos(PerformanceCounterLib,int,int,ReadOnlySpan`1):ref
           2 ( 3.17% of base) : System.Data.SqlClient.dasm - TdsParserStateObject:IncrementAndObtainOpenResultCount(SqlInternalTransaction):int:this
           2 ( 0.97% of base) : System.Private.Xml.dasm - XmlQueryType:CreateTypeCodeDerivation():BitMatrix
           1 ( 0.13% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeTablesHeader(BlobBuilder,MetadataSizes):this
           3 ( 0.12% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceNamedTypeSymbol:MakeDeclaredBases(ConsList`1,DiagnosticBag):Tuple`2:this
           2 ( 0.08% of base) : System.Configuration.ConfigurationManager.dasm - BaseConfigurationRecord:InitConfigFromFile():this
           1 ( 0.07% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeMetadataHeader(BlobBuilder,MetadataSizes):this
           2 ( 0.07% of base) : System.Private.Xml.dasm - DecimalFormatter:.ctor(String,DecimalFormat):this
           1 ( 0.07% of base) : Microsoft.CodeAnalysis.dasm - MetadataWriter:SerializeMetadataTables(BlobBuilder,MetadataSizes,int,int):this
           1 ( 0.05% of base) : System.Collections.Concurrent.dasm - ConcurrentDictionary`2:System.Collections.IDictionary.set_Item(Object,Object):this (7 methods)
Top method improvements (percentages):
          -2 (-20.00% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Directive:get_Kind():ushort:this
         -14 (-20.00% of base) : System.Collections.dasm - SortedDictionary`2:get_Count():int:this (7 methods)
          -2 (-20.00% of base) : System.Data.OleDb.dasm - OleDbInfoMessageEventArgs:get_ErrorCode():int:this
          -2 (-20.00% of base) : System.Private.DataContractSerialization.dasm - XmlBinaryReader:SkipNodeType():this
          -2 (-20.00% of base) : System.Private.Xml.dasm - XmlTextReaderImpl:get_LineNumber():int:this
          -2 (-20.00% of base) : System.Private.Xml.dasm - XmlTextReaderImpl:get_LinePosition():int:this
         -21 (-18.75% of base) : System.Collections.dasm - SortedDictionary`2:CopyTo(ref,int):this (7 methods)
          -2 (-18.18% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - BoundToQueryableCollectionConversion:get_ExpressionSymbol():Symbol:this
          -2 (-18.18% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Parser:GetBinaryOperatorHelper(SyntaxToken):ushort
          -2 (-18.18% of base) : System.Private.CoreLib.dasm - ConstructorBuilder:GetModule():Module:this
          -2 (-18.18% of base) : System.Private.CoreLib.dasm - EnumBuilder:get_Module():Module:this
          -2 (-18.18% of base) : System.Private.CoreLib.dasm - FieldBuilder:get_Module():Module:this
          -2 (-18.18% of base) : System.Private.CoreLib.dasm - GenericTypeParameterBuilder:get_Module():Module:this
          -2 (-18.18% of base) : System.Private.CoreLib.dasm - MethodBuilder:get_Module():Module:this
          -2 (-18.18% of base) : System.Private.CoreLib.dasm - PropertyBuilder:get_Module():Module:this
          -6 (-18.18% of base) : System.Private.CoreLib.dasm - CompareInfo:CompareOrdinalIgnoreCase(String,String):int
          -4 (-17.39% of base) : System.Private.CoreLib.dasm - String:EqualsOrdinalIgnoreCase(String,String):bool
          -4 (-17.39% of base) : System.Private.CoreLib.dasm - TypeBuilder:get_SyncRoot():Object:this
         -28 (-16.00% of base) : System.Collections.dasm - Node:Split4Node():this (7 methods)
         -28 (-16.00% of base) : System.Collections.dasm - Node:Merge2Nodes():this (7 methods)
8023 total methods with Code Size differences (8013 improved, 10 regressed), 236907 unchanged.

erozenfeld · 2020-02-21T06:32:21Z

Benchmarks diffs (x64 pmi):

PMI CodeSize Diffs for benchstones and benchmarks game in F:\runtime\artifacts\tests\coreclr\Windows_NT.x64.Release for  default jit
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: -195 (-0.04% of base)
    diff is an improvement.
Top file improvements (bytes):
        -146 : V8\Crypto\Crypto\Crypto.dasm (-0.38% of base)
         -14 : BenchmarksGame\fasta\fasta-1\fasta-1.dasm (-0.10% of base)
          -7 : BenchmarksGame\spectralnorm\spectralnorm-3\spectralnorm-3.dasm (-0.39% of base)
          -6 : Linq\Linq\Linq.dasm (-0.02% of base)
          -6 : BenchmarksGame\binarytrees\binarytrees-5\binarytrees-5.dasm (-0.21% of base)
          -6 : BenchmarksGame\k-nucleotide\k-nucleotide-9\k-nucleotide-9.dasm (-0.06% of base)
          -4 : SIMD\RayTracer\RayTracer\RayTracer.dasm (-0.02% of base)
          -2 : Bytemark\Bytemark\Bytemark.dasm (-0.00% of base)
          -2 : Span\Indexer\Indexer.dasm (-0.02% of base)
          -1 : Roslyn\CscBench\CscBench.dasm (-0.02% of base)
          -1 : BenchmarksGame\reverse-complement\reverse-complement-6\reverse-complement-6.dasm (-0.02% of base)
11 total files with Code Size differences (11 improved, 0 regressed), 71 unchanged.
Top method regressions (bytes):
          11 ( 2.10% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:isProbablePrime(int):bool:this
          10 ( 5.10% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:getLowestSetBit():int:this
           2 ( 1.05% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:modInt(int):int:this
Top method improvements (bytes):
         -14 (-0.31% of base) : BenchmarksGame\fasta\fasta-1\fasta-1.dasm - <TransformQueue>d__8`2:MoveNext():bool:this (7 methods)
         -12 (-1.80% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:subTo(BigInteger,BigInteger):this
         -12 (-2.42% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:bitwiseTo(BigInteger,BinOpInt,BigInteger):this
         -12 (-1.80% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:addTo(BigInteger,BigInteger):this
         -11 (-1.33% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:fromString(String,int):this
         -11 (-1.59% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:squareTo(BigInteger):this
          -9 (-0.44% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:divRemTo(BigInteger,BigInteger,BigInteger):this
          -8 (-1.76% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:multiplyLowerTo(BigInteger,int,BigInteger):this
          -7 (-1.09% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:fromByteArray(ref):this
          -7 (-1.10% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:toByteArray():ref:this
          -6 (-1.20% of base) : BenchmarksGame\k-nucleotide\k-nucleotide-9\k-nucleotide-9.dasm - <>c__DisplayClass13_0:<count4>b__4(ref):String:this
          -6 (-2.06% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:am3(BigInteger,int,int,BigInteger,int,int,int):int
          -6 (-0.70% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:toString(int):String:this
          -6 (-4.72% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:compareTo(BigInteger):int:this
          -6 (-2.17% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:intValue():int:this
          -4 (-1.40% of base) : BenchmarksGame\spectralnorm\spectralnorm-3\spectralnorm-3.dasm - Approximate:run():this
          -4 (-0.29% of base) : SIMD\RayTracer\RayTracer\RayTracer.dasm - RayTracerBench:RenderLoop(int):this
          -4 (-0.92% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:multiplyTo(BigInteger,BigInteger):this
          -4 (-1.22% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:multiplyUpperTo(BigInteger,int,BigInteger):this
          -3 (-1.97% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:PrintArray(String):this
Top method regressions (percentages):
          10 ( 5.10% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:getLowestSetBit():int:this
          11 ( 2.10% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:isProbablePrime(int):bool:this
           2 ( 1.05% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:modInt(int):int:this
Top method improvements (percentages):
          -6 (-4.72% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:compareTo(BigInteger):int:this
          -2 (-2.94% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:isEven():bool:this
          -2 (-2.86% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:byteValue():ubyte:this
          -2 (-2.86% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:shortValue():ushort:this
         -12 (-2.42% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:bitwiseTo(BigInteger,BinOpInt,BigInteger):this
          -2 (-2.25% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:signum():int:this
          -2 (-2.22% of base) : BenchmarksGame\spectralnorm\spectralnorm-3\spectralnorm-3.dasm - Approximate:MultiplyAtAv(ref,ref,ref):this
          -3 (-2.21% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:dRShiftTo(int,BigInteger):this
          -6 (-2.17% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:intValue():int:this
          -6 (-2.06% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:am3(BigInteger,int,int,BigInteger,int,int,int):int
          -3 (-2.04% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:bitCount():int:this
          -3 (-1.97% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:PrintArray(String):this
          -3 (-1.95% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:dLShiftTo(int,BigInteger):this
          -2 (-1.82% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:clamp():this
         -12 (-1.80% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:subTo(BigInteger,BigInteger):this
         -12 (-1.80% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:addTo(BigInteger,BigInteger):this
          -8 (-1.76% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:multiplyLowerTo(BigInteger,int,BigInteger):this
          -2 (-1.69% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:copyTo(BigInteger):this
         -11 (-1.59% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:squareTo(BigInteger):this
          -2 (-1.48% of base) : V8\Crypto\Crypto\Crypto.dasm - BigInteger:testBit(int):bool:this
54 total methods with Code Size differences (51 improved, 3 regressed), 1839 unchanged.

erozenfeld · 2020-02-21T06:38:58Z

Typical diff (from System.Data.SqlClient.dll):

; Assembly listing for method MemoryRecordBuffer:GetBoolean(SmiEventSink,int):bool:this
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
;
; Lcl frame size = 40

G_M45197_IG01:
       sub      rsp, 40
						;; bbWeight=1    PerfScore 0.25
G_M45197_IG02:
       mov      rax, gword ptr [rcx+8]
       cmp      r8d, dword ptr [rax+8]
       jae      SHORT G_M45197_IG04
       movsxd   rdx, r8d
       mov      rax, gword ptr [rax+8*rdx+16]
-      mov      edx, dword ptr [rax]
       movzx    rax, byte  ptr [rax+32]
+					;; bbWeight=1    PerfScore 9.25
-					;; bbWeight=1    PerfScore 11.25
G_M45197_IG03:
       add      rsp, 40
       ret      
						;; bbWeight=1    PerfScore 1.25
G_M45197_IG04:
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
						;; bbWeight=0    PerfScore 0.00

-; Total bytes of code 39, prolog size 4, PerfScore 16.65, (MethodHash=34c94f73) for method MemoryRecordBuffer:GetBoolean(SmiEventSink,int):bool:this
+; Total bytes of code 37, prolog size 4, PerfScore 14.45, (MethodHash=34c94f73) for method MemoryRecordBuffer:GetBoolean(SmiEventSink,int):bool:this

erozenfeld · 2020-02-21T06:39:55Z

@dotnet/jit-contrib @AndyAyersMS PTAL

sandreenko · 2020-02-21T08:35:40Z

src/coreclr/src/jit/morph.cpp

            else
            {
                // thisPtr = COMMA(deref("this"), "this")
-                GenTree* ind = gtNewOperNode(GT_IND, TYP_INT, thisPtr);


Was it important that sometimes we were dereferencing TYP_INT and sometimes TYP_I_IMPL?

AndyAyersMS

Similar question as @sandreenko -- what factors come into play when choosing BYTE for the type used in GT_NULLCHECK?

The non-struct case in box removal might warrant similar treatment, but I suspect boxing from scalar indirs is pretty rare.

Otherwise, looks good.

sandreenko · 2020-02-21T20:21:41Z

This change will fix half cases from #32248 (when NULL_CHECK is followed by IND and optAssertionProp_Ind doesn't match them).

erozenfeld · 2020-02-21T20:53:48Z

The type we set on GT_NULLCHECK currently doesn't matter, it's ignored in codegen:

runtime/src/coreclr/src/jit/codegenxarch.cpp

Lines 4049 to 4056 in 87bb243

    
           void CodeGen::genCodeForNullCheck(GenTreeOp* tree) 
        
           { 
        
               assert(tree->OperIs(GT_NULLCHECK)); 
        
               assert(tree->gtOp1->isUsedFromReg()); 
        
               regNumber reg = genConsumeReg(tree->gtOp1); 
        
               GetEmitter()->emitIns_AR_R(INS_cmp, EA_4BYTE, reg, reg, 0); 
        
           }

runtime/src/coreclr/src/jit/codegenarmarch.cpp

Lines 1546 to 1559 in 8079a27

    
           void CodeGen::genCodeForNullCheck(GenTreeOp* tree) 
        
           { 
        
               assert(tree->OperIs(GT_NULLCHECK)); 
        
               assert(!tree->gtOp1->isContained()); 
        
               regNumber addrReg = genConsumeReg(tree->gtOp1); 
        
           #ifdef TARGET_ARM64 
        
               regNumber targetReg = REG_ZR; 
        
           #else 
        
               regNumber targetReg = tree->GetSingleTempReg(); 
        
           #endif 
        
               GetEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, targetReg, addrReg, 0); 
        
           }

sandreenko

LGTM

erozenfeld · 2020-02-21T23:44:29Z

The non-struct case in box removal might warrant similar treatment, but I suspect boxing from scalar indirs is pretty rare.

I tried that when copySrc->gtOper == GT_IND || copySrc->gtOper == GT_FIELD and got very few new diffs, some of them regressions when we can't eliminate the nullcheck.
So, I'll leave the primitive case as is.

erozenfeld · 2020-02-22T00:06:46Z

@AndyAyersMS I answered your comments.

Use GT_NULLCHECK for unconsumed indirections.

7ade972

All unconsumed indirections are now GT_NULLCHECKs, which allows optFoldNullChecks to remove more redundant null checks.

Dotnet-GitSync-Bot added the area-Meta label Feb 21, 2020

erozenfeld requested a review from AndyAyersMS February 21, 2020 06:39

erozenfeld added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-Meta labels Feb 21, 2020

sandreenko reviewed Feb 21, 2020

View reviewed changes

AndyAyersMS reviewed Feb 21, 2020

View reviewed changes

sandreenko approved these changes Feb 21, 2020

View reviewed changes

AndyAyersMS approved these changes Feb 22, 2020

View reviewed changes

erozenfeld merged commit d872a66 into dotnet:master Feb 22, 2020

ghost locked as resolved and limited conversation to collaborators Dec 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use GT_NULLCHECK for unconsumed indirections.#32641

Use GT_NULLCHECK for unconsumed indirections.#32641
erozenfeld merged 1 commit intodotnet:masterfrom
erozenfeld:NullCheckForUnconsumedIndirs

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

sandreenko Feb 21, 2020

Uh oh!

AndyAyersMS left a comment

Uh oh!

sandreenko commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

sandreenko left a comment

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

sandreenko Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS left a comment

Choose a reason for hiding this comment

Uh oh!

sandreenko commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

sandreenko left a comment

Choose a reason for hiding this comment

Uh oh!

erozenfeld commented Feb 21, 2020

Uh oh!

erozenfeld commented Feb 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants