Implement System.Array functions used by MD arrays as intrinsics #60816

BruceForstall · 2021-10-24T23:12:04Z

Specifically, implement:

GetLength
GetLowerBound
GetUpperBound

These are implemented as intrinsics for MD arrays only, when given
a constant dimension argument that is less than the array rank (that is,
in range).

Thus, exceptional cases fall back to the IL code which throws an exception
as appropriate.

Example x64 codegen for 2-D int[,] array (instead of call or inlined call):

GetLength(1):

mov      r9d, dword ptr [rcx+20]

GetLowerBound(1):

mov      r9d, dword ptr [rcx+28]

GetUpperBound(1):

mov      r9d, dword ptr [rcx+28]
mov      r10d, dword ptr [rcx+20]
lea      r10d, [r9+r10-1]

ghost · 2021-10-24T23:12:12Z

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

Issue Details

Specifically, implement:

get_Rank
GetLength
GetLowerBound
GetUpperBound

get_Rank is implemented for both SZ and MD arrays, and replaces the result
with a constant. It falls back to IL for generic System.Array types.

The others are implemented as intrinsics for MD arrays only, when given
a constant dimension argument that is less than the array rank (that is,
in range).

Thus, exceptional cases fall back to the IL code which throws an exception
as appropriate.

Example x64 codegen for 2-D int[,] array (instead of call or inlined call):

GetLength(1):

mov      r9d, dword ptr [rcx+20]

GetLowerBound(1):

mov      r9d, dword ptr [rcx+28]

GetUpperBound(1):

mov      r9d, dword ptr [rcx+28]
mov      r10d, dword ptr [rcx+20]
lea      r10d, [r9+r10-1]

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

BruceForstall · 2021-10-24T23:16:02Z

The recently added MDGeneralArray benchmark shows 4x speedsups:

Method	Job	Toolchain	Mean	Error	StdDev	Median	Min	Max	Ratio	Allocated
MDGeneralArray	Job-RYCWUF	\runtime2\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\CoreRun.exe	55.47 ms	0.584 ms	0.546 ms	55.45 ms	54.67 ms	56.58 ms	1.00	8 KB
MDGeneralArray	Job-CLIYYD	\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\CoreRun.exe	14.61 ms	0.106 ms	0.099 ms	14.62 ms	14.40 ms	14.79 ms	0.26	8 KB

MDGeneralArray2	Job-RYCWUF	\runtime2\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\CoreRun.exe	55.30 ms	0.552 ms	0.516 ms	55.24 ms	54.64 ms	56.74 ms	1.00	9 KB
MDGeneralArray2	Job-CLIYYD	\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\CoreRun.exe	14.68 ms	0.145 ms	0.136 ms	14.63 ms	14.49 ms	15.00 ms	0.27	8 KB

This is no surprise, as the loops being measured look like:

for (int i = s.GetLowerBound(0); i <= s.GetUpperBound(0); i++) {
    for (int j = s.GetLowerBound(1); j <= s.GetUpperBound(1); j++) {
        for (int k = s.GetLowerBound(2); k <= s.GetUpperBound(2); k++) {
            if (s[i,j,k] != d[i,j,k]) {
                return false;
            }
        }
    }
}

src/coreclr/jit/importer.cpp

BruceForstall · 2021-10-24T23:19:38Z

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

            var td = HandleToObject(cls) as ArrayType;
-            Debug.Assert(td != null);


It looks like crossgen2 was too restrictive: the VM code passes returns zero if it's not a MD or SZ array.

BruceForstall · 2021-10-24T23:20:11Z

@dotnet/jit-contrib PTAL
cc @jkotas

BruceForstall · 2021-10-24T23:22:46Z

/azp run runtime-coreclr outerloop

azure-pipelines · 2021-10-24T23:23:13Z

Azure Pipelines successfully started running 1 pipeline(s).

src/coreclr/jit/importer.cpp

BruceForstall · 2021-10-25T01:44:56Z

I do not think Array.Rank is used on hot paths and/or with concrete array types. It may not be worth the trouble to treat it as JIT intrinsic.

Good point. The JIT inlines and optimizes the function call anyway for the general case. Will remove the intrinsic handling.

BruceForstall · 2021-10-25T03:48:32Z

/azp run runtime-coreclr outerloop

azure-pipelines · 2021-10-25T03:48:53Z

Azure Pipelines successfully started running 1 pipeline(s).

EgorBo · 2021-10-25T11:49:14Z

Good to see improvements in this area, I think I saw a couple of public benchmarks where .NET was in a very bad place because author decided to benchmark multi-dimension arrays 🙂 Any spmi diffs? just to make sure this area is well covered (tests)

BruceForstall · 2021-10-25T17:14:34Z

Any spmi diffs? just to make sure this area is well covered (tests)

There are a few tests that cover MD arrays, such as my newly added MDGeneralArray. And I've been using a little test program as well. However, no SPMI diffs, but that's because the new NamedIntrinsic and library attributions changes the JIT-EE interface. There are jit-diff diffs on the tests that use these functions.

AndyAyersMS

Changes LGTM.

One somewhat tangential question.

src/coreclr/jit/importer.cpp

AndyAyersMS · 2021-10-25T22:16:04Z

src/coreclr/jit/ee_il_dll.cpp

+//    See the comments at the definition of CORINFO_Array for a description of how arrays are laid out in memory.
+//
+// static
+unsigned Compiler::eeGetArrayDataOffset()


I realize the code below is pre-existing but does this work right when cross-targeting (say 64 bit hosted 32 bit targeted)?

Seems like it must, somehow.

Well, OFFSETOF__CORINFO_Array__data is ifdef'ed on TARGET_64BIT but everything else is target/bitness-independent, and we build compile-time target-bitness-specific JITs, so seems like it's all set up to work.

Thanks. I missed that this is not offsetof(CORINFO_Array, i1Elems) or similar, but a separately defined thing.

Specifically, implement: - get_Rank - GetLength - GetLowerBound - GetUpperBound get_Rank is implemented for both SZ and MD arrays, and replaces the result with a constant. It falls back to IL for generic System.Array types. The others are implemented as intrinsics for MD arrays only, when given a constant dimension argument that is less than the array rank (that is, in range). Thus, exceptional cases fall back to the IL code which throws an exception as appropriate. Example x64 codegen for 2-D `int[,]` array (instead of call or inlined call): GetLength(1): ``` mov r9d, dword ptr [rcx+20] ``` GetLowerBound(1): ``` mov r9d, dword ptr [rcx+28] ``` GetUpperBound(1): ``` mov r9d, dword ptr [rcx+28] mov r10d, dword ptr [rcx+20] lea r10d, [r9+r10-1] ```

BruceForstall requested a review from MichalStrehovsky as a code owner October 24, 2021 23:12

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

BruceForstall commented Oct 24, 2021

View reviewed changes

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

BruceForstall commented Oct 24, 2021

View reviewed changes

BruceForstall requested a review from AndyAyersMS October 24, 2021 23:19

jkotas reviewed Oct 25, 2021

View reviewed changes

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

jkotas reviewed Oct 25, 2021

View reviewed changes

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

BruceForstall mentioned this pull request Oct 25, 2021

RyuJIT: Improving multi-dimensional array accesses #60785

Closed

AndyAyersMS approved these changes Oct 25, 2021

View reviewed changes

BruceForstall added 2 commits October 25, 2021 16:08

Updated for code review comments

2ace1b8

BruceForstall force-pushed the MDFunctionIntrinsics branch from e83af0b to 2ace1b8 Compare October 25, 2021 23:09

BruceForstall merged commit 13d6458 into dotnet:main Oct 26, 2021

BruceForstall deleted the MDFunctionIntrinsics branch October 26, 2021 01:42

BruceForstall mentioned this pull request Oct 28, 2021

Assertion failed 'op1 != nullptr' #60957

Closed

ghost locked as resolved and limited conversation to collaborators Nov 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement System.Array functions used by MD arrays as intrinsics #60816

Implement System.Array functions used by MD arrays as intrinsics #60816

BruceForstall commented Oct 24, 2021 •

edited

Loading

ghost commented Oct 24, 2021

BruceForstall commented Oct 24, 2021

BruceForstall Oct 24, 2021

BruceForstall commented Oct 24, 2021

BruceForstall commented Oct 24, 2021

azure-pipelines bot commented Oct 24, 2021

BruceForstall commented Oct 25, 2021

BruceForstall commented Oct 25, 2021

azure-pipelines bot commented Oct 25, 2021

EgorBo commented Oct 25, 2021

BruceForstall commented Oct 25, 2021

AndyAyersMS left a comment

AndyAyersMS Oct 25, 2021

BruceForstall Oct 25, 2021 •

edited

Loading

AndyAyersMS Oct 25, 2021

		var td = HandleToObject(cls) as ArrayType;
		Debug.Assert(td != null);

Implement System.Array functions used by MD arrays as intrinsics #60816

Implement System.Array functions used by MD arrays as intrinsics #60816

Conversation

BruceForstall commented Oct 24, 2021 • edited Loading

ghost commented Oct 24, 2021

BruceForstall commented Oct 24, 2021

BruceForstall Oct 24, 2021

Choose a reason for hiding this comment

BruceForstall commented Oct 24, 2021

BruceForstall commented Oct 24, 2021

azure-pipelines bot commented Oct 24, 2021

BruceForstall commented Oct 25, 2021

BruceForstall commented Oct 25, 2021

azure-pipelines bot commented Oct 25, 2021

EgorBo commented Oct 25, 2021

BruceForstall commented Oct 25, 2021

AndyAyersMS left a comment

Choose a reason for hiding this comment

AndyAyersMS Oct 25, 2021

Choose a reason for hiding this comment

BruceForstall Oct 25, 2021 • edited Loading

Choose a reason for hiding this comment

AndyAyersMS Oct 25, 2021

Choose a reason for hiding this comment

BruceForstall commented Oct 24, 2021 •

edited

Loading

BruceForstall Oct 25, 2021 •

edited

Loading