Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GT_NULLCHECK for unconsumed indirections. #32641

Merged
merged 1 commit into from Feb 22, 2020

Conversation

erozenfeld
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

@dotnet/jit-contrib @AndyAyersMS PTAL

@erozenfeld 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

// COMMA(COMMA(tmp = "this", deref(tmp)), tmp)
thisPtr = gtNewOperNode(GT_COMMA, vt, asg, gtNewLclvNode(lclNum, vt));
}
else
{
// thisPtr = COMMA(deref("this"), "this")
GenTree* ind = gtNewOperNode(GT_IND, TYP_INT, thisPtr);
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor

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

@erozenfeld
Copy link
Member Author

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

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);
}

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);
}

Copy link
Contributor

@sandreenko sandreenko left a comment

Choose a reason for hiding this comment

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

LGTM

@erozenfeld
Copy link
Member Author

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
Copy link
Member Author

@AndyAyersMS I answered your comments.

@erozenfeld erozenfeld merged commit d872a66 into dotnet:master Feb 22, 2020
@msftbot msftbot bot locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants