Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Track single def locals in importer #21251

Merged
merged 2 commits into from
Nov 29, 2018

Conversation

AndyAyersMS
Copy link
Member

Two separate commits (currently).

First just refactors existing use of lvSingleDef and is no diff.

Second repurposes lvSingleDef early to keep track of locals that are amenable to type updates, and then adds some type propagation to the late devirtualization callback.

Since this information isn't used early on in the jit, defer setting this bit
until later. (a subsequent change will repurpose this bit and use it early too).
And use this information to enable type updates during late devirtualization.
This gets some cases where the exact type materializes late (perhaps via a
chain of copies).
@AndyAyersMS
Copy link
Member Author

@briansull PTAL
cc @dotnet/jit-contrib

Diffs from second commit, mainly more devirtualization:

PMI Diffs for System.Private.CoreLib.dll, framework assemblies for x64 default jit
Summary:
(Lower is better)
Total bytes of diff: -14278 (-0.04% of base)
    diff is an improvement.
Top file regressions by size (bytes):
          13 : Microsoft.CSharp.dasm (0.00% of base)
          10 : System.Collections.Immutable.dasm (0.00% of base)
           8 : Microsoft.DotNet.InternalAbstractions.dasm (0.16% of base)
           2 : NuGet.Packaging.Core.dasm (0.02% of base)
Top file improvements by size (bytes):
       -3224 : System.Linq.Parallel.dasm (-0.29% of base)
       -2733 : System.Private.Xml.dasm (-0.08% of base)
       -1461 : System.Data.Common.dasm (-0.11% of base)
        -912 : Microsoft.DotNet.ProjectModel.dasm (-0.43% of base)
        -813 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (-0.03% of base)
60 total files with size differences (56 improved, 4 regressed), 69 unchanged.
Top method regressions by size (bytes):
         135 ( 8.27% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - VisualBasicCompilation:GetExtensionAttributeConstructor(byref):ref:this
          28 ( 1.29% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - ExpressionLambdaRewriter:VisitLambdaInternal(ref,ref):ref:this
          26 ( 4.66% of base) : Microsoft.CodeAnalysis.dasm - SuppressMessageAttributeState:DecodeLocalSuppressMessageAttributes(ref,ref):ref
          14 ( 1.58% of base) : System.Threading.Tasks.Dataflow.dasm - <>c:<.ctor>b__9_6(ref,ref):this (10 methods)
          13 ( 0.66% of base) : Microsoft.CSharp.dasm - SymbolTable:SetParameterAttributes(ref,ref,int)
Top method improvements by size (bytes):
        -288 (-12.20% of base) : Microsoft.DotNet.ProjectModel.dasm - LockFileReader:ReadObject(ref,ref):ref (5 methods)
        -264 (-11.67% of base) : NuGet.ProjectModel.dasm - LockFileFormat:ReadObject(ref,ref):ref (5 methods)
        -260 (-19.05% of base) : System.Linq.Parallel.dasm - CountAggregationOperator`1:InternalAggregate(byref):int:this (5 methods)
        -260 (-18.91% of base) : System.Linq.Parallel.dasm - LongCountAggregationOperator`1:InternalAggregate(byref):long:this (5 methods)
        -260 (-16.83% of base) : System.Linq.Parallel.dasm - AnyAllSearchOperator`1:Aggregate():bool:this (5 methods)
Top method regressions by size (percentage):
         135 ( 8.27% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - VisualBasicCompilation:GetExtensionAttributeConstructor(byref):ref:this
           3 ( 6.00% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - CtfString:Read(ref):ref:this
           8 ( 5.93% of base) : Microsoft.DotNet.InternalAbstractions.dasm - FileWrapper:CreateEmptyFile(ref):this
          13 ( 5.70% of base) : System.Collections.Immutable.dasm - ImmutableArray`1:System.Collections.IStructuralEquatable.GetHashCode(ref):int:this (5 methods)
           2 ( 5.13% of base) : Microsoft.CodeAnalysis.CSharp.dasm - ReferenceManager:WeakIdentityPropertiesEquivalent(ref,ref):bool:this
Top method improvements by size (percentage):
         -43 (-25.15% of base) : System.Data.Common.dasm - ADP:IsCatchableOrSecurityExceptionType(ref):bool
         -54 (-25.12% of base) : System.Data.Common.dasm - ADP:IsCatchableExceptionType(ref):bool
         -52 (-21.94% of base) : Microsoft.CodeAnalysis.dasm - ExceptionHandlerContainerScope:FreeBasicBlocks():this
         -35 (-19.55% of base) : System.Net.Primitives.dasm - Marvin:GenerateSeed():long
         -35 (-19.55% of base) : System.Private.Xml.dasm - Marvin:GenerateSeed():long
474 total methods with size differences (428 improved, 46 regressed), 190802 unchanged.

#endif

varDsc->lvIsPtr = 1;
varDsc->lvIsPtr = 1;

Choose a reason for hiding this comment

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

Do we still have varDsc->lvSingleDef here?
It is important to mark the 'this' pointer as a single def variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

it is set later on (as you note below).


// Set initial value for lvSingleDef for explicit and implicit
// argument locals as they are "defined" on entry.
varDsc->lvSingleDef = varDsc->lvIsParam;

Choose a reason for hiding this comment

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

Is this where we get the 'this' pointers lcSingleDef set to true?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

@briansull
Copy link

You may be able to close Issue #17949 after this change.

@AndyAyersMS AndyAyersMS merged commit 40bf810 into dotnet:master Nov 29, 2018
@AndyAyersMS AndyAyersMS deleted the TrackSingleDefLocalsInImporter branch November 29, 2018 08:32
if (!varDsc->lvClassIsExact && isNewClass)
{
// Todo: improve this analysis by adding a new jit interface method
DWORD newAttrs = info.compCompHnd->getClassAttribs(clsHnd);
Copy link
Member

Choose a reason for hiding this comment

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

Computing class attribs is relatively expensive operation. It computes a ton more than just the CORINFO_FLG_SHAREDINST flag that you care about here. It would be better to change mergeClasses to return NULL when any of the types is shared.

@AndyAyersMS
Copy link
Member Author

Sure. These type updates shouldn't happen all that often but moving the check should be simpler and faster.

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Defer setting lvSingleDef until lvaMarkLocalVars

Since this information isn't used early on in the jit, defer setting this bit
until later.

* Track single-def locals in the importer

and use this information to enable type updates during late devirtualization.
This gets some cases where the exact type materializes late (perhaps via a
chain of copies).


Commit migrated from dotnet/coreclr@40bf810
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants