[clr-interp] Implement array support for reference and value types #116384

kotlarmilos · 2025-06-06T17:29:35Z

Description

This PR adds support for array operations in the interpreter, including both ref and value types.

Changes

Implemented the following opcodes:
- LDELEM_REF, LDELEM_VT
- STELEM_REF, STELEM_VT, STELEM_VT_NOREF
- LDELEMA, LDELEMA_REF
Added support for calling constructors of MD arrays

Out-of-scope

LDELEMA_MD and multi-dimensional array getters will be implemented as follow-up.

dotnet-policy-service · 2025-06-06T17:31:13Z

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

jkotas · 2025-06-06T19:46:37Z

src/coreclr/interpreter/compiler.cpp

+                        break;
+                    }
+
+                    assert(!"Array constructor with <= 1 args is not allowed");


It is not possible to call array constructor with 1 arg in C#, but it is valid in IL. I am sure we will have some test cases for it,

Ok, removed the assert.

jkotas · 2025-06-06T19:48:56Z

src/coreclr/vm/interpexec.cpp

+                    // TODO: Stack slot is 8 bytes aligned and
+                    // AllocateArrayEx expects the dimensions to be in int32_t array
+                    // so we need to convert the stack slots to int32_t
+                    int32_t* dims = (int32_t*)alloca(rank * sizeof(int32_t));


alloca will overflow the stack when called in the loop. Is it what the TODO above is about? I think this should just be moved to a helper method, so that the alloca buffer gets de-allocated once the opcode executes.

Thanks, I’ve moved it to a helper function. An alternative would be to pack argCalls into an int32_t* array, but that would require a series of mov and shift opcodes.

jkotas · 2025-06-06T19:49:46Z

src/coreclr/vm/interpexec.cpp

@@ -1252,6 +1252,31 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr

                    goto CALL_INTERP_SLOT;
                }
+                case INTOP_NEWOBJ_ARR:


I would call the opcode INTOP_NEW_MDARR to make it clear that it allocates multi-dimensional arrays.

Updated to INTOP_NEWMDARR to match INTOP_NEWARR.

src/coreclr/vm/interpexec.cpp

jkotas · 2025-06-06T19:54:42Z

src/coreclr/vm/interpexec.cpp

+                    ip += 5;
+                    break;
+                }
+                case INTOP_LDELEMA1:


What is the 1 suffix trying to say? I think this should be INTOP_LDELEMA.

This might be a mono-ism, the mono interp has an opcode named this

Mono has three different opcodes for different situations:

LDELEMA_TC is the most generic one with type checks for reference types (see the LDELEMA_REF suggestion in comments below)

LDELEMA and LDELEMA1 for arrays of value types, LDELEMA1 seems to be the specialized variant for SZARRAYs (0-based, 1-rank)

I'd personally prefer to use an unified convention across opcodes like this. This could be LDELEMA (for SZARRAY), LDELEMA_MD (for all other arrays) and LDELEMA_REF (for arrays of reference types).

We have some prior art in CoreCLR for treating SZARRAY as just "array" and using a prefix/suffix for other arrays. I don't have any preference for a specific naming convention as long as it's consistent between the opcodes.

Thanks, I agree with LDELEMA/LDELEMA_REF/LDELEMA_MD suggestion.

src/coreclr/vm/interpexec.cpp

jkotas · 2025-06-07T19:19:47Z

src/coreclr/vm/interpexec.cpp

+                    if (elemRef != NULL)
+                    {
+                        MethodTable* arrayElemMT = (MethodTable*)pMethod->pDataItems[ip[4]];
+                        if (arrayElemMT != NULL && !ObjIsInstanceOf(OBJECTREFToObject(elemRef), arrayElemMT))


ObjIsInstanceOf can trigger GC, so the object references have to be protected around the call; or re-fetched from the interpterer stack after the call. The latter is probably more efficient.

janvorli

LGTM, thank you!

…reclr-interp-array-ref

Implement missing ref array support

44244db

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 6, 2025

dotnet-policy-service bot assigned kotlarmilos Jun 6, 2025

kotlarmilos added this to the 10.0.0 milestone Jun 6, 2025

kotlarmilos added area-CodeGen-Interpreter-coreclr NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 6, 2025

kotlarmilos mentioned this pull request Jun 2, 2025

CoreCLR Interpreter #112158

Open

65 tasks

jkotas reviewed Jun 6, 2025

View reviewed changes

src/coreclr/vm/interpexec.cpp Show resolved Hide resolved

jkotas reviewed Jun 6, 2025

View reviewed changes

src/coreclr/vm/interpexec.cpp Show resolved Hide resolved

Implement CreateMultiDimArray and kArrayTypeMismatchException checks

90910db

jkotas reviewed Jun 7, 2025

View reviewed changes

src/coreclr/vm/interpexec.cpp Outdated Show resolved Hide resolved

jkotas reviewed Jun 7, 2025

View reviewed changes

src/coreclr/vm/interpexec.cpp Outdated Show resolved Hide resolved

jkotas reviewed Jun 7, 2025

View reviewed changes

kotlarmilos added 2 commits June 9, 2025 08:53

Remove TestArrayAddress in favor of other tests

6300673

Fix kArrayTypeMismatchException checks

fac735c

kotlarmilos marked this pull request as ready for review June 9, 2025 07:31

kotlarmilos requested review from BrzVlad and janvorli as code owners June 9, 2025 07:31

build-analysis bot mentioned this pull request Jun 9, 2025

CI flakiness: mono interpreter build getting killed #114123

Open

kotlarmilos removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 9, 2025

kotlarmilos changed the title ~~[WIP][clr-interp] Implement array support for reference and value types~~ [clr-interp] Implement array support for reference and value types Jun 9, 2025

janvorli approved these changes Jun 9, 2025

View reviewed changes

Merge branch 'main' of github.com:kotlarmilos/runtime into feature/co…

7b6552f

…reclr-interp-array-ref

janvorli merged commit b581381 into dotnet:main Jun 9, 2025
102 checks passed

github-actions bot locked and limited conversation to collaborators Jul 10, 2025

[clr-interp] Implement array support for reference and value types #116384

[clr-interp] Implement array support for reference and value types #116384

Conversation

kotlarmilos commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Out-of-scope

Uh oh!

dotnet-policy-service bot commented Jun 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipnavara Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipnavara Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kotlarmilos commented Jun 6, 2025 •

edited

Loading

filipnavara Jun 7, 2025 •

edited

Loading

filipnavara Jun 7, 2025 •

edited

Loading