JIT: OSR jitstress fixes; enable struct promotion #65903

AndyAyersMS · 2022-02-25T20:26:08Z

Fix OSR jitstress failures by disabling STRESS_LCL_FLD for two cases:

for Tier0 methods with patchpoints (because patchpoint info generation
does not produce the right offsets when locals are padded)
for OSR locals in OSR methods (because they live on the Tier0 frame and
so can't have their storage altered.

Enable struct promotion for OSR locals. In most cases this ends up being
dependent promotion. Enabling independent promotion will take more work.

Fix OSR jitstress failures by disabling STRESS_LCL_FLD for two cases: * for Tier0 methods with patchpoints (because patchpoint info generation does not produce the right offsets when locals are padded) * for OSR locals in OSR methods (because they live on the Tier0 frame and so can't have their storage altered. Enable struct promotion for OSR locals. In most cases this ends up being dependent promotion. Enabling independent promotion will take more work.

ghost · 2022-02-25T20:26:17Z

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix OSR jitstress failures by disabling STRESS_LCL_FLD for two cases:

for Tier0 methods with patchpoints (because patchpoint info generation
does not produce the right offsets when locals are padded)
for OSR locals in OSR methods (because they live on the Tier0 frame and
so can't have their storage altered.

Enable struct promotion for OSR locals. In most cases this ends up being
dependent promotion. Enabling independent promotion will take more work.

Author:	AndyAyersMS
Assignees:	AndyAyersMS
Labels:	`area-CodeGen-coreclr`
Milestone:	-

AndyAyersMS · 2022-02-25T20:28:40Z

@BruceForstall PTAL
cc @dotnet/jit-contrib

Expect diffs in asp.net for OSR methods (if the current collection is viable). If not I will post my local diffs.

This fixes the ~20% regression seen in BinaryTrees_2 from the performance repo, as a key field is now tracked and so doesn't hold onto a large amount of data and alter GC behavior.

See #33658 (comment) and subsequent comments for context.

AndyAyersMS · 2022-02-25T21:07:27Z

Jitstress OSR fixes were validated over in #65675 (though it is hard to tell given the CI issues in the last testing cycle).

I can't easily run jitstress + OSR on this PR, but will run OSR stress.

AndyAyersMS · 2022-02-25T21:19:06Z

/azp run runtime-jit-experimental

azure-pipelines · 2022-02-25T21:19:21Z

Azure Pipelines successfully started running 1 pipeline(s).

src/coreclr/jit/lclvars.cpp

Co-authored-by: Bruce Forstall <[email protected]>

AndyAyersMS · 2022-02-25T23:09:49Z

/azp run runtime-jit-experimental

azure-pipelines · 2022-02-25T23:10:05Z

Azure Pipelines successfully started running 1 pipeline(s).

AndyAyersMS · 2022-02-26T01:18:10Z

Opened #65914 for the asserts seen in Libraries Test Run checked coreclr Linux_musl arm Debug.

AndyAyersMS · 2022-02-26T02:04:22Z

Don't see any asp.net diffs from the spmi job. Here's what I saw locally.

(Lower is better)

Total bytes of base: 19953402 (overridden on cmd)
Total bytes of diff: 19951960 (overridden on cmd)
Total bytes of delta: -1442 (-0.01 % of base)
    diff is an improvement.
    relative diff is an improvement.


Top file regressions (bytes):
         599 : 50820.dasm (6.25% of base)
         392 : 51251.dasm (9.06% of base)
         314 : 18671.dasm (39.30% of base)
         197 : 29574.dasm (5.18% of base)
         130 : 51296.dasm (4.41% of base)
         118 : 27578.dasm (2.43% of base)
         113 : 20108.dasm (6.32% of base)
          72 : 51363.dasm (5.87% of base)
          68 : 18112.dasm (5.98% of base)
          64 : 9341.dasm (2.80% of base)
          63 : 57675.dasm (5.33% of base)
          63 : 56214.dasm (5.09% of base)
          60 : 51059.dasm (6.29% of base)
          53 : 58689.dasm (6.14% of base)
          50 : 57799.dasm (3.22% of base)
          46 : 51464.dasm (3.36% of base)
          40 : 7630.dasm (17.78% of base)
          35 : 58765.dasm (3.97% of base)
          34 : 53367.dasm (5.76% of base)
          33 : 53013.dasm (1.35% of base)

Top file improvements (bytes):
        -332 : 8788.dasm (-41.09% of base)
        -275 : 51454.dasm (-22.90% of base)
        -141 : 10883.dasm (-11.20% of base)
        -138 : 18147.dasm (-21.36% of base)
        -113 : 53372.dasm (-4.84% of base)
        -112 : 8891.dasm (-15.47% of base)
        -100 : 50491.dasm (-11.31% of base)
         -93 : 25754.dasm (-5.42% of base)
         -93 : 63154.dasm (-5.46% of base)
         -93 : 63160.dasm (-5.53% of base)
         -93 : 63737.dasm (-5.44% of base)
         -93 : 25810.dasm (-5.35% of base)
         -93 : 59556.dasm (-5.43% of base)
         -93 : 6092.dasm (-4.31% of base)
         -93 : 6095.dasm (-4.28% of base)
         -93 : 59555.dasm (-5.39% of base)
         -93 : 63738.dasm (-5.39% of base)
         -92 : 10878.dasm (-15.33% of base)
         -74 : 56172.dasm (-2.60% of base)
         -68 : 49237.dasm (-3.98% of base)

203 total files with Code Size differences (118 improved, 85 regressed), 55 unchanged.

Top method regressions (bytes):
         599 ( 6.25% of base) : 50820.dasm - RegexInterpreter:Go():this
         392 ( 9.06% of base) : 51251.dasm - SqlQueryParser:ParseRawQuery(NpgsqlCommand,NpgsqlBatchCommand,bool,bool):this
         314 (39.30% of base) : 18671.dasm - AnsiParser:Parse(String):this
         197 ( 5.18% of base) : 29574.dasm - <WriteHeadersAsync>d__56:MoveNext():this
         130 ( 4.41% of base) : 51296.dasm - <<Write>g__WriteExecute|94_0>d:MoveNext():this
         118 ( 2.43% of base) : 27578.dasm - XmlTextReaderImpl:ParseAttributeValueSlow(int,ushort,NodeData):this
         113 ( 6.32% of base) : 20108.dasm - <StopAsync>d__14:MoveNext():this
          72 ( 5.87% of base) : 51363.dasm - <QueryAsync>d__33`1:MoveNext():this
          68 ( 5.98% of base) : 18112.dasm - ConcurrentDictionary`2:TryAddInternal(__Canon,Nullable`1,ChangeTokenInfo,bool,bool,byref):bool:this
          64 ( 2.80% of base) : 9341.dasm - ValueStringBuilder:AppendFormatHelper(IFormatProvider,String,ParamsArray):this
          63 ( 5.33% of base) : 57675.dasm - EntityMaterializerInjectingExpressionVisitor:MaterializeEntity(EntityShaperExpression,ParameterExpression,ParameterExpression,ParameterExpression,ParameterExpression):Expression:this
          63 ( 5.09% of base) : 56214.dasm - TypeMappingInfo:.ctor(IReadOnlyList`1,Nullable`1,Nullable`1,Nullable`1,Nullable`1):this
          60 ( 6.29% of base) : 51059.dasm - RegexCompiler:<EmitFindFirstChar>g__EmitFixedSet_LeftToRight|123_2(byref):this
          53 ( 6.14% of base) : 58689.dasm - SslStream:WriteSingleChunk(ReadOnlyMemory`1,CancellationToken):ValueTask:this
          50 ( 3.22% of base) : 57799.dasm - StackSpiller:RewriteSwitchExpression(Expression,int):Result:this
          46 ( 3.36% of base) : 51464.dasm - ConcurrentDictionary`2:TryAddInternal(ServiceCacheKey,Nullable`1,GeneratedMethod,bool,bool,byref):bool:this
          40 (17.78% of base) : 7630.dasm - MemoryExtensions:IndexOfAnyProbabilistic(byref,int,byref,int):int
          35 ( 3.97% of base) : 58765.dasm - <Microsoft-EntityFrameworkCore-Infrastructure-IResettableService-ResetStateAsync>d__76:MoveNext():this
          34 ( 5.76% of base) : 53367.dasm - ArraySortHelper`1:PickPivotAndPartition(Span`1,Comparison`1):int
          33 ( 1.35% of base) : 53013.dasm - <ForceAuthenticationAsync>d__166`1:MoveNext():this

Top method improvements (bytes):
        -332 (-41.09% of base) : 8788.dasm - LowLevelLifoSemaphore:Wait(int,bool):bool:this
        -275 (-22.90% of base) : 51454.dasm - HttpHeaders:ParseConnection(HttpHeaders):int
        -141 (-11.20% of base) : 10883.dasm - HttpParser`1:ParseHeaders(ParsingAdapter,byref):bool:this
        -138 (-21.36% of base) : 18147.dasm - HostBuilder:CreateServiceProvider():this
        -113 (-4.84% of base) : 53372.dasm - HttpMethodMatcherPolicy:GetEdges(IReadOnlyList`1):IReadOnlyList`1:this
        -112 (-15.47% of base) : 8891.dasm - TextInfo:ChangeCaseCommon(String):String:this
        -100 (-11.31% of base) : 50491.dasm - RegexCharClass:ToStringClass(bool,byref):this
         -93 (-5.42% of base) : 25754.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.46% of base) : 63154.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.53% of base) : 63160.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.44% of base) : 63737.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.35% of base) : 25810.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.43% of base) : 59556.dasm - WorkerThread:WorkerThreadStart()
         -93 (-4.31% of base) : 6092.dasm - WorkerThread:WorkerThreadStart()
         -93 (-4.28% of base) : 6095.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.39% of base) : 59555.dasm - WorkerThread:WorkerThreadStart()
         -93 (-5.39% of base) : 63738.dasm - WorkerThread:WorkerThreadStart()
         -92 (-15.33% of base) : 10878.dasm - HttpParser`1:ParseRequestLine(ParsingAdapter,ReadOnlySpan`1):this
         -74 (-2.60% of base) : 56172.dasm - SharedTableConvention:TryUniquifyTableNames(IConventionModel,Dictionary`2,int)
         -68 (-3.98% of base) : 49237.dasm - WorkerThread:WorkerThreadStart()

Top method regressions (percentages):
         314 (39.30% of base) : 18671.dasm - AnsiParser:Parse(String):this
          28 (21.21% of base) : 8478.dasm - CustomAttribute:GetCustomAttributes(RuntimeModule,int,int,RuntimeType):ref
          22 (18.33% of base) : 9390.dasm - RuntimeConstructorInfo:Invoke(int,Binder,ref,CultureInfo):Object:this
          22 (18.33% of base) : 8853.dasm - RuntimeMethodInfo:Invoke(Object,int,Binder,ref,CultureInfo):Object:this
          40 (17.78% of base) : 7630.dasm - MemoryExtensions:IndexOfAnyProbabilistic(byref,int,byref,int):int
          20 (14.49% of base) : 51051.dasm - <>c:<CharsToStringClass>b__95_0(Span`1,ValueTuple`2):this
          19 (14.29% of base) : 8955.dasm - Sha1ForNonSecretPurposes:Append(ReadOnlySpan`1):this
          16 (13.68% of base) : 27515.dasm - XmlTextReaderImpl:EatPreamble():this
          14 ( 9.66% of base) : 55381.dasm - DelayedConventionScope:Run(ConventionDispatcher):this
         392 ( 9.06% of base) : 51251.dasm - SqlQueryParser:ParseRawQuery(NpgsqlCommand,NpgsqlBatchCommand,bool,bool):this
          32 ( 8.82% of base) : 10058.dasm - SparseArrayBuilder`1:CopyTo(ref,int,int):this
           6 ( 8.70% of base) : 7534.dasm - PathInternal:IsEffectivelyEmpty(ReadOnlySpan`1):bool
          18 ( 7.76% of base) : 52318.dasm - DefaultModelMetadataProvider:GetMetadataForProperties(Type):IEnumerable`1:this
          13 ( 7.69% of base) : 52386.dasm - OrderedEnumerable`1:ToList():List`1:this
           7 ( 7.37% of base) : 7804.dasm - Utf8JsonReader:SkipWhiteSpace():this
          23 ( 6.73% of base) : 18407.dasm - LoggerRuleSelector:Select(LoggerFilterOptions,Type,String,byref,byref)
          26 ( 6.57% of base) : 50979.dasm - PoolManager:TryGetValue(String,byref):bool
         113 ( 6.32% of base) : 20108.dasm - <StopAsync>d__14:MoveNext():this
          60 ( 6.29% of base) : 51059.dasm - RegexCompiler:<EmitFindFirstChar>g__EmitFixedSet_LeftToRight|123_2(byref):this
         599 ( 6.25% of base) : 50820.dasm - RegexInterpreter:Go():this

Top method improvements (percentages):
        -332 (-41.09% of base) : 8788.dasm - LowLevelLifoSemaphore:Wait(int,bool):bool:this
        -275 (-22.90% of base) : 51454.dasm - HttpHeaders:ParseConnection(HttpHeaders):int
         -63 (-22.74% of base) : 29218.dasm - Uri:CheckForUnicodeOrEscapedUnreserved(String):bool
         -22 (-22.22% of base) : 11046.dasm - ThreadCounts:InterlockedSetNumThreadsGoal(short):ThreadCounts:this
        -138 (-21.36% of base) : 18147.dasm - HostBuilder:CreateServiceProvider():this
        -112 (-15.47% of base) : 8891.dasm - TextInfo:ChangeCaseCommon(String):String:this
         -92 (-15.33% of base) : 10878.dasm - HttpParser`1:ParseRequestLine(ParsingAdapter,ReadOnlySpan`1):this
         -44 (-15.12% of base) : 53472.dasm - FastPathTokenizer:Tokenize(String,Span`1):int
         -36 (-15.00% of base) : 18014.dasm - HostBuilder:BuildHostConfiguration():this
         -40 (-14.93% of base) : 18049.dasm - HostBuilder:BuildAppConfiguration():this
         -36 (-14.88% of base) : 7966.dasm - Hashtable:rehash(int):this
         -28 (-14.14% of base) : 7056.dasm - AllowedBmpCodePointsBitmap:ForbidUndefinedCharacters():this
         -51 (-13.42% of base) : 9548.dasm - StringBuilder:Replace(ushort,ushort,int,int):StringBuilder:this
         -32 (-13.01% of base) : 1280.dasm - AllowedBmpCodePointsBitmap:ForbidUndefinedCharacters():this
         -35 (-12.68% of base) : 9637.dasm - CompareInfo:LastIndexOf(String,String,int,int,int):int:this
         -38 (-12.62% of base) : 50870.dasm - StringBuilder:CopyTo(int,Span`1,int):this
         -23 (-11.86% of base) : 28638.dasm - TransformBuilder:BuildInternal(RouteConfig,ClusterConfig):StructuredTransformer:this
         -22 (-11.58% of base) : 51356.dasm - RuntimeTypeHandle:CopyRuntimeTypeHandles(ref,byref):ref
         -12 (-11.54% of base) : 8951.dasm - WorkerThread:ShouldStopProcessingWorkNow(PortableThreadPool):bool
        -100 (-11.31% of base) : 50491.dasm - RegexCharClass:ToStringClass(bool,byref):this

203 total methods with Code Size differences (118 improved, 85 regressed), 55 unchanged.

There are around 1350 OSR methods in this collection, so about 10% were impacted.

AndyAyersMS · 2022-02-26T03:07:16Z

Some pre-existing stress failures in jit-experimental -- these are not mainline OSR so will investigate them separately.

AndyAyersMS · 2022-02-26T03:43:34Z

FYI the failure in GitHub_21990 is that we need to do stack probing in a funclet frame because under current arm64 OSR, funclet frames must pad with the Tier0 frame size to ensure they get the same PSPSym offset as the OSR frame (which incorporate the Tier0 frame).

G_M57283_IG13:              ;; offset=00ECH
        D2939C09          mov     x9, #0x9ce0
        CB2963FF          sub     sp, sp, x9
        A9007BFD          stp     fp, lr, [sp]                      ** blows up here **
        A90253F3          stp     x19, x20, [sp,#32]
        D2939C02          mov     x2, #0x9ce0
        8B0203A3          add     x3, fp, x2
        F9000FE3          str     x3, [sp,#24]
						;; bbWeight=0    PerfScore 0.00
G_M57283_IG14:              ;; offset=0108H
        10FFFE60          adr     x0, [G_M57283_IG11]
						;; bbWeight=0    PerfScore 0.00
G_M57283_IG15:              ;; offset=010CH
        A94253F3          ldp     x19, x20, [sp,#32]
        A9407BFD          ldp     fp, lr, [sp]
        D2939C09          mov     x9, #0x9ce0
        8B2963FF          add     sp, sp, x9
        D65F03C0          ret     lr

This undoes part of dotnet#65903. OSR methods can't rely solely on their own analysis for struct promotion as they only see parts of methods.

This undoes part of #65903. OSR methods can't rely solely on their own analysis for struct promotion as they only see parts of methods.

ghost assigned AndyAyersMS Feb 25, 2022

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 25, 2022

AndyAyersMS requested a review from BruceForstall February 25, 2022 20:30

AndyAyersMS mentioned this pull request Feb 25, 2022

On Stack Replacement Next Steps #33658

Open

72 tasks

BruceForstall approved these changes Feb 25, 2022

View reviewed changes

src/coreclr/jit/lclvars.cpp Outdated Show resolved Hide resolved

Fix typo

2b244cc

Co-authored-by: Bruce Forstall <[email protected]>

AndyAyersMS merged commit c8da2fd into dotnet:main Feb 26, 2022

AndyAyersMS mentioned this pull request Mar 24, 2022

OSR stress failure in System.Diagnostics.DiagnosticsSource.Tests #67078

Closed

AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this pull request Mar 25, 2022

JIT: block some struct promotion for OSR

b32e80a

This undoes part of dotnet#65903. OSR methods can't rely solely on their own analysis for struct promotion as they only see parts of methods.

AndyAyersMS mentioned this pull request Mar 25, 2022

JIT: block some struct promotion for OSR #67131

Merged

AndyAyersMS added a commit that referenced this pull request Mar 25, 2022

JIT: block some struct promotion for OSR (#67131)

6627183

This undoes part of #65903. OSR methods can't rely solely on their own analysis for struct promotion as they only see parts of methods.

ghost locked as resolved and limited conversation to collaborators Mar 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: OSR jitstress fixes; enable struct promotion #65903

JIT: OSR jitstress fixes; enable struct promotion #65903

AndyAyersMS commented Feb 25, 2022

ghost commented Feb 25, 2022

AndyAyersMS commented Feb 25, 2022 •

edited

Loading

AndyAyersMS commented Feb 25, 2022

AndyAyersMS commented Feb 25, 2022

azure-pipelines bot commented Feb 25, 2022

AndyAyersMS commented Feb 25, 2022

azure-pipelines bot commented Feb 25, 2022

AndyAyersMS commented Feb 26, 2022

AndyAyersMS commented Feb 26, 2022 •

edited

Loading

AndyAyersMS commented Feb 26, 2022

AndyAyersMS commented Feb 26, 2022

JIT: OSR jitstress fixes; enable struct promotion #65903

JIT: OSR jitstress fixes; enable struct promotion #65903

Conversation

AndyAyersMS commented Feb 25, 2022

ghost commented Feb 25, 2022

AndyAyersMS commented Feb 25, 2022 • edited Loading

AndyAyersMS commented Feb 25, 2022

AndyAyersMS commented Feb 25, 2022

azure-pipelines bot commented Feb 25, 2022

AndyAyersMS commented Feb 25, 2022

azure-pipelines bot commented Feb 25, 2022

AndyAyersMS commented Feb 26, 2022

AndyAyersMS commented Feb 26, 2022 • edited Loading

AndyAyersMS commented Feb 26, 2022

AndyAyersMS commented Feb 26, 2022

AndyAyersMS commented Feb 25, 2022 •

edited

Loading

AndyAyersMS commented Feb 26, 2022 •

edited

Loading