[Perf] Changes at 12/28/2021 6:18:01 PM

[performanceautofiler]

Run Information

Architecture	arm64
OS	Windows 10.0.19041
Baseline	d094a34a4d555d3aef434bc87ad3fefe15d83e08
Compare	15e95fc44be0ea444ac145b3cc2d4207fa1a6fef
Diff	Diff

Regressions in System.Numerics.Tests.Perf_Matrix4x4

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
InequalityOperatorBenchmark - Duration of single invocation	11.74 ns	13.11 ns	1.12	0.12	False

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Numerics.Tests.Perf_Matrix4x4*'

Payloads

Baseline
Compare

Histogram

System.Numerics.Tests.Perf_Matrix4x4.InequalityOperatorBenchmark

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 13.109460078459284 > 12.288147412937622.
IsChangePoint: Marked as a change because one of 12/28/2021 8:42:38 AM, 1/5/2022 11:37:43 PM falls between 12/28/2021 8:42:38 AM and 1/5/2022 11:37:43 PM.
IsRegressionStdDev: Marked as regression because -19.598191341561044 (T) = (0 -13.018293385362945) / Math.Sqrt((0.01873680514312647 / (8)) + (0.06181210359770353 / (25))) is less than -2.0395134463927462 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (8) + (25) - 2, .025) and -0.11664210781814657 = (11.658429584748447 - 13.018293385362945) / 11.658429584748447 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

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

Issue Details

---

Run Information

Architecture	arm64
OS	Windows 10.0.19041
Baseline	d094a34a4d555d3aef434bc87ad3fefe15d83e08
Compare	15e95fc44be0ea444ac145b3cc2d4207fa1a6fef
Diff	Diff

Regressions in System.Numerics.Tests.Perf_Matrix4x4

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
InequalityOperatorBenchmark - Duration of single invocation	11.74 ns	13.11 ns	1.12	0.12	False

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Numerics.Tests.Perf_Matrix4x4*'

Payloads

Baseline
Compare

Histogram

System.Numerics.Tests.Perf_Matrix4x4.InequalityOperatorBenchmark

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 13.109460078459284 > 12.288147412937622.
IsChangePoint: Marked as a change because one of 12/28/2021 8:42:38 AM, 1/5/2022 11:37:43 PM falls between 12/28/2021 8:42:38 AM and 1/5/2022 11:37:43 PM.
IsRegressionStdDev: Marked as regression because -19.598191341561044 (T) = (0 -13.018293385362945) / Math.Sqrt((0.01873680514312647 / (8)) + (0.06181210359770353 / (25))) is less than -2.0395134463927462 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (8) + (25) - 2, .025) and -0.11664210781814657 = (11.658429584748447 - 13.018293385362945) / 11.658429584748447 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Author:	performanceautofiler[bot]
Assignees:	-
Labels:	arch-arm64, os-windows, tenet-performance, tenet-performance-benchmarks, area-CodeGen-coreclr, untriaged
Milestone:	-

[kunalspathak]

Mostly from #61030
@echesakov

[echesakov]

Taking a look

[echesakov]

The resulting diffs look fine

diff --git a/base/20869.dasm b/diff/20869.dasm
index 98bf79427da..c182fa087f7 100644
--- a/base/20869.dasm
+++ b/diff/20869.dasm
@@ -24,35 +24,27 @@ G_M11226_IG02:        ; gcrefRegs=0000 {}, byrefRegs=0000 {}, byref
             bl      System.Numerics.Matrix4x4:get_Identity():System.Numerics.Matrix4x4
             add     x8, fp, #144	// [V03 tmp2]
             bl      System.Numerics.Matrix4x4:get_Identity():System.Numerics.Matrix4x4
-            ldp     x0, x1, [fp,#208]	// [V02 tmp1]
-            stp     x0, x1, [fp,#80]	// [V04 tmp3]
-            ldp     x0, x1, [fp,#224]	// [V02 tmp1+0x10]
-            stp     x0, x1, [fp,#96]	// [V04 tmp3+0x10]
-            ldp     x0, x1, [fp,#240]	// [V02 tmp1+0x20]
-            stp     x0, x1, [fp,#112]	// [V04 tmp3+0x20]
-            ldp     x0, x1, [fp,#0xd1ffab1e]	// [V02 tmp1+0x30]
-            stp     x0, x1, [fp,#128]	// [V04 tmp3+0x30]
-            ldp     x0, x1, [fp,#144]	// [V03 tmp2]
-            stp     x0, x1, [fp,#16]	// [V05 tmp4]
-            ldp     x0, x1, [fp,#160]	// [V03 tmp2+0x10]
-            stp     x0, x1, [fp,#32]	// [V05 tmp4+0x10]
-            ldp     x0, x1, [fp,#176]	// [V03 tmp2+0x20]
-            stp     x0, x1, [fp,#48]	// [V05 tmp4+0x20]
-            ldp     x0, x1, [fp,#192]	// [V03 tmp2+0x30]
-            stp     x0, x1, [fp,#64]	// [V05 tmp4+0x30]
+            ldp     q16, q17, [fp,#208]
+            stp     q16, q17, [fp,#80]
+            ldp     q16, q17, [fp,#240]
+            stp     q16, q17, [fp,#112]
+            ldp     q16, q17, [fp,#144]
+            stp     q16, q17, [fp,#16]
+            ldp     q16, q17, [fp,#176]
+            stp     q16, q17, [fp,#48]
             add     x0, fp, #80	// [V04 tmp3]
             ; byrRegs +[x0]
             add     x1, fp, #16	// [V05 tmp4]
             ; byrRegs +[x1]
             bl      System.Numerics.Matrix4x4:op_Inequality()
             ; byrRegs -[x0-x1]
-						;; bbWeight=1    PerfScore 37.00
+						;; bbWeight=1    PerfScore 17.00
 G_M11226_IG03:        ; , epilog, nogc, extend
             ldp     fp, lr, [sp],#0xd1ffab1e
             ret     lr
 						;; bbWeight=1    PerfScore 2.00
 
-; Total bytes of code 108, prolog size 8, PerfScore 51.30, instruction count 27, allocated bytes for code 108 (MethodHash=f11fd425) for method System.Numerics.Tests.Perf_Matrix4x4:InequalityOperatorBenchmark():bool:this
+; Total bytes of code 76, prolog size 8, PerfScore 28.10, instruction count 19, allocated bytes for code 76 (MethodHash=f11fd425) for method System.Numerics.Tests.Perf_Matrix4x4:InequalityOperatorBenchmark():bool:this
 ; ============================================================
 
 Unwind Info:
@@ -63,7 +55,7 @@ Unwind Info:
   E bit             : 0
   X bit             : 0
   Vers              : 0
-  Function Length   : 27 (0x0001b) Actual length = 108 (0x00006c)
+  Function Length   : 19 (0x00013) Actual length = 76 (0x00004c)
   ---- Epilog scopes ----
   ---- Scope 0
   Epilog Start Offset        : 3523193630 (0xd1ffab1e) Actual offset = 3523193630 (0xd1ffab1e) Offset from main function begin = 3523193630 (0xd1ffab1e)

Will try to repro this on Arm64 hardware.

[echesakov]

I could reproduce the regression locally

Method	Job	Toolchain	Mean	Error	StdDev	Median	Min	Max	Ratio	Allocated
InequalityOperatorBenchmark	Job-LHTOTV	\base\Core_Root\corerun.exe	11.36 ns	0.066 ns	0.058 ns	11.33 ns	11.30 ns	11.49 ns	1.00	-
InequalityOperatorBenchmark	Job-VAMQRG	\diff\Core_Root\corerun.exe	13.38 ns	0.047 ns	0.039 ns	13.38 ns	13.32 ns	13.46 ns	1.18	-

[AndyAyersMS]

Is there some reason why we can't load/store to successively increasing addresses (though I see we didn't do that before either)? Seems like it would generally be more compatible with various HW driven optimizations (like prefetching).

Also seems like a case where we should be able to avoid making the copy in the first place. Kunal has started thinking about cases like this...

[echesakov]

Is there some reason why we can't load/store to successively increasing addresses (though I see we didn't do that before either)?

The instruction sequences are coming from different IR nodes (in this case, it copies local A to local B and local C to local D). I think we can do this when coping to local (i.e. we can reorder the instructions without making the effect observable from outside of the thread). We can do this as a part of broader pipehole optimization.

[echesakov]

In this particular case, I believe the source of the regression is System.Numerics.Matrix4x4:get_Identity():System.Numerics.Matrix4x4 where ldp/stp xReg1, xReg2 were replaced with ldp/stp dReg1, dReg2

diff --git a/base.txt b/diff.txt
index 3041b88..ff5512c 100644
--- a/base.txt
+++ b/diff.txt
@@ -27,14 +27,14 @@ G_M21828_IG02:              ;; offset=0010H
         F2A4B800          movk    x0, #0x25c0 LSL #16
         F2C02660          movk    x0, #307 LSL #32
         F9400000          ldr     x0, [x0]
-        A9408801          ldp     x1, x2, [x0,#8]
-        A9000A61          stp     x1, x2, [x19]
-        A9418801          ldp     x1, x2, [x0,#24]
-        A9010A61          stp     x1, x2, [x19,#16]
-        A9428801          ldp     x1, x2, [x0,#40]
-        A9020A61          stp     x1, x2, [x19,#32]
-        A9438801          ldp     x1, x2, [x0,#56]
-        A9030A61          stp     x1, x2, [x19,#48]
+        6D40C410          ldp     d16, d17, [x0,#8]
+        6D004670          stp     d16, d17, [x19]
+        6D41C410          ldp     d16, d17, [x0,#24]
+        6D014670          stp     d16, d17, [x19,#16]
+        6D42C410          ldp     d16, d17, [x0,#40]
+        6D024670          stp     d16, d17, [x19,#32]
+        6D43C410          ldp     d16, d17, [x0,#56]
+        6D034670          stp     d16, d17, [x19,#48]
 						;; bbWeight=1    PerfScore 27.50

[kunalspathak]

In this particular case, I believe the source of the regression is System.Numerics.Matrix4x4:get_Identity():System.Numerics.Matrix4x4 where ldp/stp xReg1, xReg2 were replaced with ldp/stp dReg1, dReg2

@TamarChristinaArm - Is the copy of dReg slower than that of xReg?

[kunalspathak]

Seems to be the case: (from Arm64 optimization guide)

[echesakov]

Tweaked the algorithm slightly, so it uses integer registers for this case. And the regression went away:

Method	Job	Toolchain	Mean	Error	StdDev	Median	Min	Max	Ratio	Allocated
InequalityOperatorBenchmark	Job-EKBWAL	\base\Core_Root\corerun.exe	12.08 ns	0.038 ns	0.032 ns	12.09 ns	12.02 ns	12.16 ns	1.00	-
InequalityOperatorBenchmark	Job-JBPNAI	\diff\Core_Root\corerun.exe	12.35 ns	0.035 ns	0.029 ns	12.36 ns	12.31 ns	12.40 ns	1.02	-

I will change the register allocation strategy for a case when 16-byte SIMD instructions cannot be used for copying (i.e. as in get_Identity) and if there is not enough integer registers either fallback to using rsGetRsvdReg() or allocate an additional register (depending on if I see code size regressions in the latter case) instead of using 8-byte SIMD instructions.

[TamarChristinaArm]

Is there some reason why we can't load/store to successively increasing addresses (though I see we didn't do that before either)? Seems like it would generally be more compatible with various HW driven optimizations (like prefetching).

No, as you say, doing the increasing addresses is easier for the HW. On some cores (particularly newer big cores) this also allows the hardware to detect streaming mode for copies. Which helps it avoid polluting the cache by pulling data in just to write it out somewhere else immediately.

Also for copies, best performance is gotten when the stores are aligned, either to 16B or 32B depending on the core.
So this sequence

+            ldp     q16, q17, [fp,#208]
+            stp     q16, q17, [fp,#80]
+            ldp     q16, q17, [fp,#240]
+            stp     q16, q17, [fp,#112]
+            ldp     q16, q17, [fp,#144]
+            stp     q16, q17, [fp,#16]
+            ldp     q16, q17, [fp,#176]
+            stp     q16, q17, [fp,#48]

looks aligned to 16b so that's ok for current cores (future ones like 32b better) but would benefit from ordering the instructions.

In this particular case, I believe the source of the regression is System.Numerics.Matrix4x4:get_Identity():System.Numerics.Matrix4x4 where ldp/stp xReg1, xReg2 were replaced with ldp/stp dReg1, dReg2

@TamarChristinaArm - Is the copy of dReg slower than that of xReg?

As you've found in the guide yes, the actual load itself is as fast, but the dReg loads requires an additional cycle for the addressing mode. In e.g. ldp d0, d1, [x2, #8] it has to use an additional integer pipeline to do the offset calculation before it can do the load because the base address is an integer reg.

I will change the register allocation strategy for a case when 16-byte SIMD instructions cannot be used for copying (i.e. as in get_Identity) and if there is not enough integer registers either fallback to using rsGetRsvdReg() or allocate an additional register (depending on if I see code size regressions in the latter case) instead of using 8-byte SIMD instructions.

So the copies in the examples are sequential, I suppose it's not easy to fold them into Q sized copies? Seems like that same copy can be done using 2 Q-regs copies.

[echesakov]

So the copies in the examples are sequential, I suppose it's not easy to fold them into Q sized copies? Seems like that same copy can be done using 2 Q-regs copies.

@TamarChristinaArm I could fold them into Q sized copies but the algorithm deliberately doesn't do this since the alignments of the values in x0 and x19 here are unknown. When I tried using Q sized copies everywhere I saw some performance regressions that I thought were due to the stores not being 16B aligned.

In #61030 I allowed Q sized copies only for a local to local CopyBlock (when base registers are either sp that is 16B aligned or fp and the JIT knows distance between sp and fp)

[echesakov]

@TamarChristinaArm Maybe you can think about a better strategy for where to allow Q sized copies?

[TamarChristinaArm]

So the copies in the examples are sequential, I suppose it's not easy to fold them into Q sized copies? Seems like that same copy can be done using 2 Q-regs copies.

@TamarChristinaArm I could fold them into Q sized copies but the algorithm deliberately doesn't do this since the alignments of the values in x0 and x19 here are unknown. When I tried using Q sized copies everywhere I saw some performance regressions that I thought were due to the stores not being 16B aligned.

@echesakovMSFT The alignment shouldn't have mattered much for small copies, but different cores can be more sensitive than others sadly.

@TamarChristinaArm Maybe you can think about a better strategy for where to allow Q sized copies?

Sure! I'll take a look once I'm back in the office (I'm away the next 3 weeks). For the immediate regression your fix is correct, you should avoid using the simd regs for smaller copies and I'll get back to you on the small Q copies.

[echesakov]

@kunalspathak @AndyAyersMS Should I keep the issue open until we have validation that the regression is fixed from the CI perf run?

[kunalspathak]

Should I keep the issue open until

Not needed. :)