Skip to content

JIT: don't forward sub a volatile read tree via copy - #62224

Merged
AndyAyersMS merged 3 commits into
dotnet:mainfrom
AndyAyersMS:DontFwdSubVolatileReadByCopy
Dec 2, 2021
Merged

JIT: don't forward sub a volatile read tree via copy#62224
AndyAyersMS merged 3 commits into
dotnet:mainfrom
AndyAyersMS:DontFwdSubVolatileReadByCopy

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Dec 1, 2021

Copy link
Copy Markdown
Member

The copy may not get the same value as the original.

Closes#62048.

@AndyAyersMS
JIT: don't forward sub a volatile read tree via copy
06d566f
The copy may not get the same value as the original.
Closesdotnet#62048.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 1, 2021
@ghost

ghost commented Dec 1, 2021

Copy link
Copy Markdown

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

Issue Details

The copy may not get the same value as the original.

Closes #62048.

Author:AndyAyersMS
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@AndyAyersMS

AndyAyersMS commented Dec 1, 2021

Copy link
Copy Markdown
MemberAuthor

Some small regressions. Suspect many of the task related ones would not be there in a release SPC.

Many could be avoided if we were smarter about when we could move the tree rather than copy (say there's some unrelated statement between the volatile load relop and the jump). But we don't do enough analysis right now to prove this is safe.

aspnet.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 13868297 (overridden on cmd)
Total bytes of diff: 13868331 (overridden on cmd)
Total bytes of delta: 34 (0.00 % of base)
diff is a regression.
relative diff is a regression.
Detail diffs

Top file regressions (bytes):
6 : 18575.dasm (0.71% of base)
6 : 34950.dasm (0.71% of base)
6 : 32074.dasm (0.71% of base)
6 : 43417.dasm (0.71% of base)
5 : 16493.dasm (0.53% of base)
4 : 9653.dasm (0.27% of base)
1 : 11098.dasm (0.12% of base)
7 total files with Code Size differences (0 improved, 7 regressed), 0 unchanged.
Top method regressions (bytes):
6 ( 0.71% of base) : 18575.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
6 ( 0.71% of base) : 34950.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
6 ( 0.71% of base) : 32074.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
6 ( 0.71% of base) : 43417.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
5 ( 0.53% of base) : 16493.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
4 ( 0.27% of base) : 9653.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
1 ( 0.12% of base) : 11098.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
Top method regressions (percentages):
6 ( 0.71% of base) : 18575.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
6 ( 0.71% of base) : 34950.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
6 ( 0.71% of base) : 32074.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
6 ( 0.71% of base) : 43417.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
5 ( 0.53% of base) : 16493.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
4 ( 0.27% of base) : 9653.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
1 ( 0.12% of base) : 11098.dasm - HostingApplicationDiagnostics:BeginRequest(HttpContext,Context):this
7 total methods with Code Size differences (0 improved, 7 regressed), 0 unchanged.

benchmarks.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 12760187 (overridden on cmd)
Total bytes of diff: 12760611 (overridden on cmd)
Total bytes of delta: 424 (0.00 % of base)
diff is a regression.
relative diff is a regression.
Detail diffs

Top file regressions (bytes):
43 : 6480.dasm (1.62% of base)
22 : 12656.dasm (1.79% of base)
16 : 12686.dasm (2.61% of base)
16 : 12657.dasm (2.82% of base)
16 : 6483.dasm (0.79% of base)
16 : 14282.dasm (0.43% of base)
15 : 4547.dasm (3.50% of base)
11 : 14310.dasm (1.24% of base)
11 : 5595.dasm (4.15% of base)
11 : 5550.dasm (2.08% of base)
11 : 6497.dasm (1.40% of base)
8 : 4059.dasm (12.31% of base)
8 : 6565.dasm (2.30% of base)
8 : 11071.dasm (0.62% of base)
8 : 37187.dasm (0.40% of base)
8 : 9351.dasm (2.99% of base)
8 : 11077.dasm (12.12% of base)
8 : 4597.dasm (2.27% of base)
8 : 6186.dasm (8.99% of base)
8 : 6101.dasm (12.12% of base)
47 total files with Code Size differences (0 improved, 47 regressed), 1 unchanged.
Top method regressions (bytes):
43 ( 1.62% of base) : 6480.dasm - <ReadLineAsyncInternal>d__63:MoveNext():this
22 ( 1.79% of base) : 12656.dasm - System.Diagnostics.Tracing.EventSource:DoCommand(System.Diagnostics.Tracing.EventCommandEventArgs):this
16 ( 0.43% of base) : 14282.dasm - <ReadAsyncSlowPath>d__39:MoveNext():this
16 ( 0.79% of base) : 6483.dasm - <ReadBufferAsync>d__71:MoveNext():this
16 ( 2.61% of base) : 12686.dasm - System.Diagnostics.Tracing.EventSource:DefineEventPipeEvents():this
16 ( 2.82% of base) : 12657.dasm - System.Diagnostics.Tracing.EventSource:EnsureDescriptorsInitialized():this
15 ( 3.50% of base) : 4547.dasm - System.Threading.ManualResetEventSlim:Set(bool):this
11 ( 1.24% of base) : 14310.dasm - <ReadScatterAtOffsetSingleSyscallAsync>d__31:MoveNext():this
11 ( 2.08% of base) : 5550.dasm - System.Threading.SemaphoreSlim:WaitAsync(int,System.Threading.CancellationToken):System.Threading.Tasks.Task`1[Boolean]:this
11 ( 1.40% of base) : 6497.dasm - System.Threading.Tasks.UnwrapPromise`1[VoidTaskResult][System.Threading.Tasks.VoidTaskResult]:TrySetFromTask(System.Threading.Tasks.Task,bool):bool:this
11 ( 4.15% of base) : 5595.dasm - WorkStealingQueue:LocalPush_HandleTailOverflow():int:this
8 ( 0.40% of base) : 37187.dasm - <<CopyToAsync>g__Core|27_0>d:MoveNext():this
8 ( 0.36% of base) : 12086.dasm - <InternalReadAllBytesUnknownLengthAsync>d__71:MoveNext():this
8 ( 0.62% of base) : 11071.dasm - <ReadScatterAtOffsetMultipleSyscallsAsync>d__33:MoveNext():this
8 ( 0.55% of base) : 14081.dasm - <WaitUntilCountOrTimeoutAsync>d__31:MoveNext():this
8 ( 2.30% of base) : 6565.dasm - ConfiguredValueTaskAwaiter[ReadOnlyMemory`1][System.ReadOnlyMemory`1[System.Byte]]:GetResult():System.ReadOnlyMemory`1[Byte]:this
8 ( 3.70% of base) : 544.dasm - Enumerator[__Canon,__Canon][System.__Canon,System.__Canon]:.ctor(System.Runtime.CompilerServices.ConditionalWeakTable`2[__Canon,__Canon]):this
8 ( 2.01% of base) : 9181.dasm - FinalizationHelper[__Canon][System.__Canon]:Finalize():this
8 ( 0.60% of base) : 42192.dasm - System.Diagnostics.Tracing.EventSource:WriteEventVarargs(int,long,System.Object[]):this
8 ( 0.63% of base) : 12704.dasm - System.Diagnostics.Tracing.EventSource:WriteEventWithRelatedActivityIdCore(int,long,int,long):this
Top method regressions (percentages):
8 (12.31% of base) : 4059.dasm - System.Threading.Tasks.Task`1[Int32][System.Int32]:get_ResultOnSuccess():int:this
8 (12.12% of base) : 6101.dasm - System.Threading.Tasks.Task`1[__Canon][System.__Canon]:get_ResultOnSuccess():System.__Canon:this
8 (12.12% of base) : 11077.dasm - System.Threading.Tasks.Task`1[Int64][System.Int64]:get_ResultOnSuccess():long:this
8 ( 8.99% of base) : 6186.dasm - System.Threading.Tasks.Task`1[ValueTuple`2][System.ValueTuple`2[System.__Canon,System.__Canon]]:get_ResultOnSuccess():System.ValueTuple`2[__Canon,__Canon]:this
8 ( 7.27% of base) : 16242.dasm - System.Threading.Tasks.Task`1[LargeStructWithProperties][MicroBenchmarks.Serializers.LargeStructWithProperties]:get_ResultOnSuccess():MicroBenchmarks.Serializers.LargeStructWithProperties:this
4 ( 6.90% of base) : 5922.dasm - System.Threading.SpinLock:.ctor(bool):this
8 ( 6.06% of base) : 38737.dasm - System.Threading.CancellationTokenSource:NotifyCancellation(bool):this
8 ( 5.76% of base) : 11344.dasm - System.Threading.Tasks.Task`1[Int32][System.Int32]:GetResultCore(bool):int:this
8 ( 5.71% of base) : 10789.dasm - System.Threading.Tasks.Task`1[Boolean][System.Boolean]:GetResultCore(bool):bool:this
11 ( 4.15% of base) : 5595.dasm - WorkStealingQueue:LocalPush_HandleTailOverflow():int:this
8 ( 3.70% of base) : 544.dasm - Enumerator[__Canon,__Canon][System.__Canon,System.__Canon]:.ctor(System.Runtime.CompilerServices.ConditionalWeakTable`2[__Canon,__Canon]):this
15 ( 3.50% of base) : 4547.dasm - System.Threading.ManualResetEventSlim:Set(bool):this
8 ( 2.99% of base) : 9351.dasm - System.IO.Stream:EndRead(System.IAsyncResult):int:this
4 ( 2.82% of base) : 41797.dasm - System.Diagnostics.Tracing.EventSource:AnyEventEnabled():bool:this
16 ( 2.82% of base) : 12657.dasm - System.Diagnostics.Tracing.EventSource:EnsureDescriptorsInitialized():this
8 ( 2.66% of base) : 9350.dasm - System.IO.Stream:EndWrite(System.IAsyncResult):this
16 ( 2.61% of base) : 12686.dasm - System.Diagnostics.Tracing.EventSource:DefineEventPipeEvents():this
7 ( 2.49% of base) : 12703.dasm - System.Diagnostics.Tracing.EventSource:EnableEventForDispatcher(System.Diagnostics.Tracing.EventDispatcher,int,int,bool):bool:this
4 ( 2.40% of base) : 1452.dasm - System.Collections.Hashtable:Clear():this
8 ( 2.30% of base) : 6565.dasm - ConfiguredValueTaskAwaiter[ReadOnlyMemory`1][System.ReadOnlyMemory`1[System.Byte]]:GetResult():System.ReadOnlyMemory`1[Byte]:this
47 total methods with Code Size differences (0 improved, 47 regressed), 1 unchanged.

coreclr_tests.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 125643038 (overridden on cmd)
Total bytes of diff: 125643193 (overridden on cmd)
Total bytes of delta: 155 (0.00 % of base)
diff is a regression.
relative diff is a regression.
Detail diffs

Top file regressions (bytes):
11 : 254338.dasm (4.72% of base)
8 : 254272.dasm (5.67% of base)
8 : 257379.dasm (6.96% of base)
8 : 254292.dasm (11.43% of base)
8 : 254845.dasm (0.73% of base)
8 : 254232.dasm (5.71% of base)
8 : 254293.dasm (5.56% of base)
8 : 254305.dasm (4.28% of base)
8 : 254307.dasm (5.03% of base)
8 : 257389.dasm (6.96% of base)
8 : 254271.dasm (11.94% of base)
8 : 255050.dasm (2.64% of base)
8 : 254306.dasm (9.41% of base)
8 : 255060.dasm (5.00% of base)
8 : 257398.dasm (2.68% of base)
8 : 254829.dasm (0.67% of base)
8 : 254103.dasm (1.93% of base)
8 : 257315.dasm (2.68% of base)
8 : 254231.dasm (12.12% of base)
19 total files with Code Size differences (0 improved, 19 regressed), 0 unchanged.
Top method regressions (bytes):
11 ( 4.72% of base) : 254338.dasm - System.Threading.Tasks.SystemThreadingTasks_FutureDebugView`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 0.73% of base) : 254845.dasm - <ToBlockingEnumerable>d__3`1[Byte][System.Byte]:MoveNext():bool:this
8 ( 6.96% of base) : 257389.dasm - ConfiguredTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 257315.dasm - ConfiguredValueTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 1.93% of base) : 254103.dasm - FinalizationHelper[Byte][System.Byte]:Finalize():this
8 ( 6.96% of base) : 257379.dasm - System.Runtime.CompilerServices.TaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 257398.dasm - System.Runtime.CompilerServices.ValueTaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 (12.12% of base) : 254231.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:get_ResultOnSuccess():ubyte:this
8 ( 5.71% of base) : 254232.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:GetResultCore(bool):ubyte:this
8 (11.43% of base) : 254292.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:get_ResultOnSuccess():double:this
8 ( 5.56% of base) : 254293.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:GetResultCore(bool):double:this
8 (11.94% of base) : 254271.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:get_ResultOnSuccess():short:this
8 ( 5.67% of base) : 254272.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:GetResultCore(bool):short:this
8 ( 4.28% of base) : 254305.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 9.41% of base) : 254306.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_ResultOnSuccess():System.Numerics.Vector`1[Single]:this
8 ( 5.03% of base) : 254307.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResultCore(bool):System.Numerics.Vector`1[Single]:this
8 ( 5.00% of base) : 255060.dasm - System.Threading.Tasks.TaskToApm:End(System.IAsyncResult):System.Numerics.Vector`1[Single]
8 ( 2.64% of base) : 255050.dasm - System.Threading.Tasks.ValueTask`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 0.67% of base) : 254829.dasm - WhenAllPromise`1[Byte][System.Byte]:Invoke(System.Threading.Tasks.Task):this
Top method regressions (percentages):
8 (12.12% of base) : 254231.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:get_ResultOnSuccess():ubyte:this
8 (11.94% of base) : 254271.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:get_ResultOnSuccess():short:this
8 (11.43% of base) : 254292.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:get_ResultOnSuccess():double:this
8 ( 9.41% of base) : 254306.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_ResultOnSuccess():System.Numerics.Vector`1[Single]:this
8 ( 6.96% of base) : 257389.dasm - ConfiguredTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 6.96% of base) : 257379.dasm - System.Runtime.CompilerServices.TaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 5.71% of base) : 254232.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:GetResultCore(bool):ubyte:this
8 ( 5.67% of base) : 254272.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:GetResultCore(bool):short:this
8 ( 5.56% of base) : 254293.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:GetResultCore(bool):double:this
8 ( 5.03% of base) : 254307.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResultCore(bool):System.Numerics.Vector`1[Single]:this
8 ( 5.00% of base) : 255060.dasm - System.Threading.Tasks.TaskToApm:End(System.IAsyncResult):System.Numerics.Vector`1[Single]
11 ( 4.72% of base) : 254338.dasm - System.Threading.Tasks.SystemThreadingTasks_FutureDebugView`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 4.28% of base) : 254305.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 257315.dasm - ConfiguredValueTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 257398.dasm - System.Runtime.CompilerServices.ValueTaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.64% of base) : 255050.dasm - System.Threading.Tasks.ValueTask`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 1.93% of base) : 254103.dasm - FinalizationHelper[Byte][System.Byte]:Finalize():this
8 ( 0.73% of base) : 254845.dasm - <ToBlockingEnumerable>d__3`1[Byte][System.Byte]:MoveNext():bool:this
8 ( 0.67% of base) : 254829.dasm - WhenAllPromise`1[Byte][System.Byte]:Invoke(System.Threading.Tasks.Task):this
19 total methods with Code Size differences (0 improved, 19 regressed), 0 unchanged.

libraries.crossgen2.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 34402381 (overridden on cmd)
Total bytes of diff: 34402928 (overridden on cmd)
Total bytes of delta: 547 (0.00 % of base)
diff is a regression.
relative diff is a regression.
Detail diffs

Top file regressions (bytes):
22 : 25931.dasm (3.70% of base)
22 : 25935.dasm (1.87% of base)
18 : 25981.dasm (3.26% of base)
16 : 27249.dasm (0.89% of base)
16 : 27251.dasm (0.39% of base)
16 : 27255.dasm (0.71% of base)
15 : 34422.dasm (3.89% of base)
11 : 34200.dasm (2.69% of base)
11 : 25862.dasm (3.00% of base)
11 : 34436.dasm (15.94% of base)
11 : 46030.dasm (2.86% of base)
9 : 34173.dasm (3.32% of base)
8 : 27314.dasm (7.21% of base)
8 : 45450.dasm (5.84% of base)
8 : 33324.dasm (3.31% of base)
8 : 34301.dasm (1.78% of base)
8 : 45899.dasm (13.11% of base)
8 : 27111.dasm (0.91% of base)
8 : 27795.dasm (0.58% of base)
8 : 34565.dasm (0.70% of base)
65 total files with Code Size differences (0 improved, 65 regressed), 0 unchanged.
Top method regressions (bytes):
22 ( 1.87% of base) : 25935.dasm - System.Diagnostics.Tracing.EventSource:DoCommand(System.Diagnostics.Tracing.EventCommandEventArgs):this
22 ( 3.70% of base) : 25931.dasm - System.Diagnostics.Tracing.EventSource:EnsureDescriptorsInitialized():this
18 ( 3.26% of base) : 25981.dasm - System.Diagnostics.Tracing.EventSource:DefineEventPipeEvents():this
16 ( 0.39% of base) : 27251.dasm - <ReadAsyncInternal>d__68:MoveNext():this
16 ( 0.89% of base) : 27249.dasm - <ReadBufferAsync>d__71:MoveNext():this
16 ( 0.71% of base) : 27255.dasm - <ReadLineAsyncInternal>d__63:MoveNext():this
15 ( 3.89% of base) : 34422.dasm - System.Threading.ManualResetEventSlim:Set(bool):this
11 ( 2.69% of base) : 34200.dasm - FinalizationHelper:Finalize():this
11 ( 2.86% of base) : 46030.dasm - FinalizationHelper:Finalize():this
11 ( 3.00% of base) : 25862.dasm - System.Diagnostics.Tracing.EventListener:RemoveReferencesToListenerInEventSources(System.Diagnostics.Tracing.EventListener)
11 (15.94% of base) : 34436.dasm - System.Threading.ManualResetEventSlim:get_WaitHandle():System.Threading.WaitHandle:this
9 ( 3.32% of base) : 34173.dasm - WorkStealingQueue:LocalPush_HandleTailOverflow():int:this
8 ( 0.51% of base) : 27309.dasm - <<CopyToAsync>g__Core|27_0>d:MoveNext():this
8 ( 0.49% of base) : 32392.dasm - <<ReadAsync>g__ReadAsyncCore|41_0>d:MoveNext():this
8 ( 7.21% of base) : 27314.dasm - <>c:<RunReadWriteTaskWhenReady>b__49_0(System.Threading.Tasks.Task,System.Object):this
8 ( 0.70% of base) : 27793.dasm - <InternalReadAllBytesAsync>d__70:MoveNext():this
8 ( 0.56% of base) : 27791.dasm - <InternalReadAllBytesUnknownLengthAsync>d__71:MoveNext():this
8 ( 0.85% of base) : 27789.dasm - <InternalReadAllLinesAsync>d__75:MoveNext():this
8 ( 0.58% of base) : 27795.dasm - <InternalReadAllTextAsync>d__66:MoveNext():this
8 ( 0.91% of base) : 27111.dasm - <ReadBlockAsyncInternal>d__20:MoveNext():this
Top method regressions (percentages):
11 (15.94% of base) : 34436.dasm - System.Threading.ManualResetEventSlim:get_WaitHandle():System.Threading.WaitHandle:this
8 (13.33% of base) : 45494.dasm - System.Threading.Tasks.Task`1:get_ResultOnSuccess():int:this
8 (13.33% of base) : 45451.dasm - System.Threading.Tasks.Task`1:get_ResultOnSuccess():System.Threading.Tasks.VoidTaskResult:this
8 (13.11% of base) : 45109.dasm - System.Threading.Tasks.Task`1:get_ResultOnSuccess():bool:this
8 (13.11% of base) : 45899.dasm - System.Threading.Tasks.Task`1:get_ResultOnSuccess():long:this
8 (13.11% of base) : 33830.dasm - System.Threading.Tasks.Task`1:get_ResultOnSuccess():System.__Canon:this
8 ( 7.77% of base) : 33612.dasm - System.Threading.Tasks.Task:GetCancellationExceptionDispatchInfo():System.Runtime.ExceptionServices.ExceptionDispatchInfo:this
5 ( 7.69% of base) : 31922.dasm - System.Runtime.MemoryFailPoint:get_MemoryFailPointReservedMemory():long
8 ( 7.21% of base) : 27314.dasm - <>c:<RunReadWriteTaskWhenReady>b__49_0(System.Threading.Tasks.Task,System.Object):this
4 ( 7.02% of base) : 34280.dasm - System.Threading.SpinLock:.ctor(bool):this
8 ( 5.97% of base) : 34566.dasm - System.Threading.CancellationTokenSource:NotifyCancellation(bool):this
8 ( 5.84% of base) : 45493.dasm - System.Threading.Tasks.Task`1:GetResultCore(bool):int:this
8 ( 5.84% of base) : 45450.dasm - System.Threading.Tasks.Task`1:GetResultCore(bool):System.Threading.Tasks.VoidTaskResult:this
8 ( 5.80% of base) : 45181.dasm - System.Threading.Tasks.Task`1:GetResultCore(bool):bool:this
8 ( 5.80% of base) : 45898.dasm - System.Threading.Tasks.Task`1:GetResultCore(bool):long:this
8 ( 5.80% of base) : 33829.dasm - System.Threading.Tasks.Task`1:GetResultCore(bool):System.__Canon:this
8 ( 5.16% of base) : 33571.dasm - System.Threading.Tasks.Task:InternalCancelContinueWithInitialState():this
7 ( 4.96% of base) : 25933.dasm - System.Diagnostics.Tracing.EventSource:AnyEventEnabled():bool:this
5 ( 4.03% of base) : 33652.dasm - System.Threading.Tasks.Task:DisregardChild():this
8 ( 3.96% of base) : 30598.dasm - Enumerator:.ctor(System.Runtime.CompilerServices.ConditionalWeakTable`2[System.__Canon, System.__Canon]):this
65 total methods with Code Size differences (0 improved, 65 regressed), 0 unchanged.

libraries.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 45297619 (overridden on cmd)
Total bytes of diff: 45297782 (overridden on cmd)
Total bytes of delta: 163 (0.00 % of base)
diff is a regression.
relative diff is a regression.
Detail diffs

Top file regressions (bytes):
11 : 17917.dasm (4.72% of base)
8 : 17886.dasm (5.03% of base)
8 : 17884.dasm (4.28% of base)
8 : 17872.dasm (5.56% of base)
8 : 230486.dasm (9.64% of base)
8 : 17682.dasm (1.93% of base)
8 : 20958.dasm (6.96% of base)
8 : 17811.dasm (5.71% of base)
8 : 17851.dasm (5.67% of base)
8 : 20968.dasm (6.96% of base)
8 : 17885.dasm (9.41% of base)
8 : 18424.dasm (0.73% of base)
8 : 18408.dasm (0.67% of base)
8 : 20977.dasm (2.68% of base)
8 : 17871.dasm (11.43% of base)
8 : 17850.dasm (11.94% of base)
8 : 18629.dasm (2.64% of base)
8 : 17810.dasm (12.12% of base)
8 : 18639.dasm (5.00% of base)
8 : 20894.dasm (2.68% of base)
20 total files with Code Size differences (0 improved, 20 regressed), 0 unchanged.
Top method regressions (bytes):
11 ( 4.72% of base) : 17917.dasm - System.Threading.Tasks.SystemThreadingTasks_FutureDebugView`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 0.73% of base) : 18424.dasm - <ToBlockingEnumerable>d__3`1[Byte][System.Byte]:MoveNext():bool:this
8 ( 6.96% of base) : 20968.dasm - ConfiguredTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 20894.dasm - ConfiguredValueTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 1.93% of base) : 17682.dasm - FinalizationHelper[Byte][System.Byte]:Finalize():this
8 ( 6.96% of base) : 20958.dasm - System.Runtime.CompilerServices.TaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 20977.dasm - System.Runtime.CompilerServices.ValueTaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 9.64% of base) : 230486.dasm - System.Threading.CountdownEvent:get_WaitHandle():System.Threading.WaitHandle:this
8 (12.12% of base) : 17810.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:get_ResultOnSuccess():ubyte:this
8 ( 5.71% of base) : 17811.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:GetResultCore(bool):ubyte:this
8 (11.43% of base) : 17871.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:get_ResultOnSuccess():double:this
8 ( 5.56% of base) : 17872.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:GetResultCore(bool):double:this
8 (11.94% of base) : 17850.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:get_ResultOnSuccess():short:this
8 ( 5.67% of base) : 17851.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:GetResultCore(bool):short:this
8 ( 4.28% of base) : 17884.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 9.41% of base) : 17885.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_ResultOnSuccess():System.Numerics.Vector`1[Single]:this
8 ( 5.03% of base) : 17886.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResultCore(bool):System.Numerics.Vector`1[Single]:this
8 ( 5.00% of base) : 18639.dasm - System.Threading.Tasks.TaskToApm:End(System.IAsyncResult):System.Numerics.Vector`1[Single]
8 ( 2.64% of base) : 18629.dasm - System.Threading.Tasks.ValueTask`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 0.67% of base) : 18408.dasm - WhenAllPromise`1[Byte][System.Byte]:Invoke(System.Threading.Tasks.Task):this
Top method regressions (percentages):
8 (12.12% of base) : 17810.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:get_ResultOnSuccess():ubyte:this
8 (11.94% of base) : 17850.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:get_ResultOnSuccess():short:this
8 (11.43% of base) : 17871.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:get_ResultOnSuccess():double:this
8 ( 9.64% of base) : 230486.dasm - System.Threading.CountdownEvent:get_WaitHandle():System.Threading.WaitHandle:this
8 ( 9.41% of base) : 17885.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_ResultOnSuccess():System.Numerics.Vector`1[Single]:this
8 ( 6.96% of base) : 20968.dasm - ConfiguredTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 6.96% of base) : 20958.dasm - System.Runtime.CompilerServices.TaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 5.71% of base) : 17811.dasm - System.Threading.Tasks.Task`1[Byte][System.Byte]:GetResultCore(bool):ubyte:this
8 ( 5.67% of base) : 17851.dasm - System.Threading.Tasks.Task`1[Int16][System.Int16]:GetResultCore(bool):short:this
8 ( 5.56% of base) : 17872.dasm - System.Threading.Tasks.Task`1[Double][System.Double]:GetResultCore(bool):double:this
8 ( 5.03% of base) : 17886.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResultCore(bool):System.Numerics.Vector`1[Single]:this
8 ( 5.00% of base) : 18639.dasm - System.Threading.Tasks.TaskToApm:End(System.IAsyncResult):System.Numerics.Vector`1[Single]
11 ( 4.72% of base) : 17917.dasm - System.Threading.Tasks.SystemThreadingTasks_FutureDebugView`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 4.28% of base) : 17884.dasm - System.Threading.Tasks.Task`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 20894.dasm - ConfiguredValueTaskAwaiter[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.68% of base) : 20977.dasm - System.Runtime.CompilerServices.ValueTaskAwaiter`1[Vector`1][System.Numerics.Vector`1[System.Single]]:GetResult():System.Numerics.Vector`1[Single]:this
8 ( 2.64% of base) : 18629.dasm - System.Threading.Tasks.ValueTask`1[Vector`1][System.Numerics.Vector`1[System.Single]]:get_Result():System.Numerics.Vector`1[Single]:this
8 ( 1.93% of base) : 17682.dasm - FinalizationHelper[Byte][System.Byte]:Finalize():this
8 ( 0.73% of base) : 18424.dasm - <ToBlockingEnumerable>d__3`1[Byte][System.Byte]:MoveNext():bool:this
8 ( 0.67% of base) : 18408.dasm - WhenAllPromise`1[Byte][System.Byte]:Invoke(System.Threading.Tasks.Task):this
20 total methods with Code Size differences (0 improved, 20 regressed), 0 unchanged.

libraries_tests.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 114293816 (overridden on cmd)
Total bytes of diff: 114293856 (overridden on cmd)
Total bytes of delta: 40 (0.00 % of base)
diff is a regression.
relative diff is a regression.
Detail diffs

Top file regressions (bytes):
8 : 208234.dasm (1.44% of base)
8 : 208233.dasm (1.44% of base)
8 : 341030.dasm (2.69% of base)
8 : 341486.dasm (11.59% of base)
8 : 341109.dasm (1.44% of base)
5 total files with Code Size differences (0 improved, 5 regressed), 0 unchanged.
Top method regressions (bytes):
8 (11.59% of base) : 341486.dasm - <>c__DisplayClass6_0:<RunManualResetEventSlimTest5_Dispose_Negative>b__2():this
8 ( 1.44% of base) : 208234.dasm - System.Threading.Tasks.Tests.UnwrapTests:Generic_AttachedToParent():this
8 ( 1.44% of base) : 208233.dasm - System.Threading.Tasks.Tests.UnwrapTests:NonGeneric_AttachedToParent():this
8 ( 2.69% of base) : 341030.dasm - System.Threading.Tests.CountdownEventTests:RunCountdownEventTest1_SimpleTimeout(int)
8 ( 1.44% of base) : 341109.dasm - System.Threading.Tests.ManualResetEventSlimTests:RunManualResetEventSlimTest5_Dispose_Negative()
Top method regressions (percentages):
8 (11.59% of base) : 341486.dasm - <>c__DisplayClass6_0:<RunManualResetEventSlimTest5_Dispose_Negative>b__2():this
8 ( 2.69% of base) : 341030.dasm - System.Threading.Tests.CountdownEventTests:RunCountdownEventTest1_SimpleTimeout(int)
8 ( 1.44% of base) : 341109.dasm - System.Threading.Tests.ManualResetEventSlimTests:RunManualResetEventSlimTest5_Dispose_Negative()
8 ( 1.44% of base) : 208234.dasm - System.Threading.Tasks.Tests.UnwrapTests:Generic_AttachedToParent():this
8 ( 1.44% of base) : 208233.dasm - System.Threading.Tasks.Tests.UnwrapTests:NonGeneric_AttachedToParent():this
5 total methods with Code Size differences (0 improved, 5 regressed), 0 unchanged.

@AndyAyersMS

AndyAyersMS commented Dec 1, 2021

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib

@AndyAyersMS

AndyAyersMS commented Dec 1, 2021

Copy link
Copy Markdown
MemberAuthor

Not sure this is the best way to check for a volatile read tree -- seems like we might want to propagate this upwards or (as noted in the comment) be able to pull this fact out of the VN somehow.

SingleAccretion
SingleAccretion reviewed Dec 1, 2021
View reviewed changes
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
Comment on lines +1087 to +1093
// Note the candidateTree's result may still be live.
//
// We should only duplicate candidateTree tree if we're sure the copy
// will produce the same result. In particular if the tree contains
// volatile reads, it may not evaluate the same.
//
// Volatile reads are modelled as side effects in VN but not in the IR.

@SingleAccretionSingleAccretionDec 1, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume checking for GTF_ORDER_SIDEEFF resulted in too many (avoidable) regressions?

@AndyAyersMSAndyAyersMSDec 1, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Good question, let me check -- doing that would be simpler.

@AndyAyersMSAndyAyersMSDec 1, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

That seems to work out just as well, so will update.

@AndyAyersMS
AndyAyersMS merged commit 2a80066 into dotnet:mainDec 2, 2021
@AndyAyersMS
AndyAyersMS deleted the DontFwdSubVolatileReadByCopy branch December 2, 2021 02:34
@ghostghost locked as resolved and limited conversation to collaborators Jan 1, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@SingleAccretionSingleAccretionSingleAccretion left review comments
@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Forward substitution for relops can create new data races

3 participants

@AndyAyersMS@BruceForstall@SingleAccretion