JIT: Advance cursors through stores in strength reduction

[jakobbotsch]

This allows us to look through CSE'd locals when an IV was CSE'd.

Example diff:

private static void CopyInts(int[] arr1, int[] arr2)
{
    if (arr2.Length != arr1.Length)
        return;

    for (int i = 0; i < arr1.Length; i++)
    {
        arr2[i] = arr1[i];
    }
}

@@ -1,14 +1,16 @@
 G_M55903_IG03:  ;; offset=0x000C
-       xor      eax, eax
        test     r8d, r8d
-       jle      SHORT G_M55903_IG05
-						;; size=7 bbWeight=0.50 PerfScore 0.75
+       jle      SHORT G_M55903_IG06
+						;; size=5 bbWeight=0.50 PerfScore 0.62
 
-G_M55903_IG04:  ;; offset=0x0013
-       mov      r10d, eax
-       mov      r9d, dword ptr [rcx+4*r10+0x10]
-       mov      dword ptr [rdx+4*r10+0x10], r9d
-       inc      eax
-       cmp      r8d, eax
-       jg       SHORT G_M55903_IG04
-						;; size=20 bbWeight=3.96 PerfScore 18.81
+G_M55903_IG04:  ;; offset=0x0011
+       mov      eax, 16
+						;; size=5 bbWeight=0.25 PerfScore 0.06
+
+G_M55903_IG05:  ;; offset=0x0016
+       mov      r10d, dword ptr [rcx+rax]
+       mov      dword ptr [rdx+rax], r10d
+       add      rax, 4
+       dec      r8d
+       jne      SHORT G_M55903_IG05
+						;; size=17 bbWeight=3.96 PerfScore 17.82

arm64 especially benefits from strength reduction, e.g. a benchmark for the above:

BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Unknown processor
  Job-GXMAGJ : .NET 9.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
  Job-MLUOLU : .NET 9.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD

Method	Toolchain	Mean	Error	Ratio
CopyInts	Main	6.683 μs	0.0010 μs	1.00
CopyInts	PR	5.014 μs	0.0003 μs	0.75

[dotnet-policy-service]

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

[jakobbotsch]

FYI @AndyAyersMS ... not sure if this makes preview 7 in time.

[EgorBo]

@EgorBot -intel

using BenchmarkDotNet.Attributes;
using System.Runtime.CompilerServices;

namespace Loops
{
    public class StrengthReduction
    {
        private int[] _arrayInts;
        private int[] _dest;

        [GlobalSetup]
        public void Setup()
        {
            _arrayInts = Enumerable.Range(0, 10000).Select(i => i).ToArray();
            _dest = new int[_arrayInts.Length];
        }

        [Benchmark]
        public void CopyInts()
        {
            Copy(_dest, _arrayInts);
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        private static void Copy(int[] dest, int[] src)
        {
            if (src.Length != dest.Length)
                return;

            for (int i = 0; i < dest.Length; i++)
            {
                dest[i] = src[i];
            }
        }
    }
}

[jakobbotsch]

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[EgorBot]

Benchmark results on Intel

BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 8 logical and 4 physical cores
  Job-NORFNC : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-ACQJKF : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Method	Toolchain	Mean	Error	Ratio
CopyInts	Main	3.422 μs	0.0148 μs	1.00
CopyInts	PR	NA	NA	?
Benchmarks with issues:
StrengthReduction.CopyInts: Job-ACQJKF(Toolchain=PR)

BDN_Artifacts.zip

[AndyAyersMS]

I'm ok taking this. You have 45 mins to merge...

[EgorBot]

Benchmark results on Intel

BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 8 logical and 4 physical cores
  Job-CDNSFW : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-AWFJUW : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Method	Toolchain	Mean	Error	Ratio
CopyInts	Main	3.423 μs	0.0068 μs	1.00
CopyInts	PR	3.173 μs	0.0085 μs	0.93

BDN_Artifacts.zip

[EgorBot]

Benchmark results on Arm64

BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Unknown processor
  Job-GXMAGJ : .NET 9.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD
  Job-MLUOLU : .NET 9.0.0 (42.42.42.42424), Arm64 RyuJIT AdvSIMD

Method	Toolchain	Mean	Error	Ratio
CopyInts	Main	6.683 μs	0.0010 μs	1.00
CopyInts	PR	5.014 μs	0.0003 μs	0.75

BDN_Artifacts.zip

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[jakobbotsch]

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS (you already approved, but this has changed a bit since then).

Diffs. In some cases a size regression, but almost always a perfscore improvement. For example, here's a canonical size regression:

[jakobbotsch]

This whole syntactic treatment of GT_ARR_ADDR to figure out what address ranges are inside arrays is unfortunate. It's pretty fragile and leads e.g. to #108706 not just working out properly. Not sure how to improve this much however -- maybe we can keep some breadcrumbs about known managed ranges during the cursor advancement and query it any time we need to prove something here.

[AndyAyersMS]

For advanced loop opts (parallelization, vectorization) it's not uncommon to create a "raised up" representation that describes the array element as an affine function of the IVs, so maybe we can look into something like this?

[jakobbotsch]

Sounds like that could be a direction to go, although I guess it would mean that we go from INDEX_ADDR to expanded nodes in morph and then back to some equivalent of INDEX_ADDR as part of these opts. But maybe that's fine (and perhaps the structure could be recovered on VNs, similar to the comment on ParseArrayAddress suggests).

[jakobbotsch]

Ping @AndyAyersMS for a re-look

[AndyAyersMS]

For advanced loop opts (parallelization, vectorization) it's not uncommon to create a "raised up" representation that describes the array element as an affine function of the IVs, so maybe we can look into something like this?

[AndyAyersMS]

We could set the ssa def trees to null, as that is a valid state (see eg #108548), though it might require some tweaking if a lot of code assumes a null def tree means initial value.

[jakobbotsch]

Or perhaps in some way remove the def from the SSA information, though I suppose that requires updating SSA numbers which requires finding all uses, so not really tractable with our representation.

This seems to work fine for now though, so I'll probably keep it as is, but it definitely falls into our SSA updating woes...