[mono] Add runtime support for Swift calling convention on x64 and arm64 #94764

kotlarmilos · 2023-11-15T10:01:16Z

Description

This PR introduces basic support for the Swift calling convention in the mini and interpreter backends on x64 and arm64 architectures. When a C# function calls a Swift library, it is expected that the self context is stored in a dedicated register. Similarly, when a Swift function returns, the error is supposed to be placed in a specific register.

In the method prolog, the self pointer is transferred to a specific register, and in the method epilog, the error register is moved to the stack and returned as an out argument. LLVM support will be addressed in a subsequent PR since the Mono runtime currently doesn't compile native wrappers using LLVM on Apple platforms.

Contributes to #94081 and #64215.

…/kotlarmilos/runtime into poc/swift-interop-direct-pinvoke

The Swift error argument is not present in the P/Invoke signature, but the context register value can be changed by Swift. Therefore, it must be preserved during the P/Invoke call by updating the used_int_regs.

matouskozak · 2024-01-25T15:54:47Z

/azp run runtime-extra-platforms

azure-pipelines · 2024-01-25T15:55:13Z

Azure Pipelines successfully started running 1 pipeline(s).

…llConvSwift signature

lambdageek

lgtm. some minor cleanup nits. and two questions:

I'm not sure why the backend looks for both SwiftError and SwiftError* arguments.
Should we prevent these pinvoke wrappers from being inlined?

src/mono/mono/metadata/marshal.c

lambdageek · 2024-01-30T14:51:40Z

src/mono/mono/metadata/marshal.c

+					} else if (!m_class_is_enumtype (param_klass) && !m_class_is_primitive (param_klass) && m_class_get_this_arg (param_klass)->type != MONO_TYPE_FNPTR) {
+						swift_error_args = swift_self_args = 0;


are blittable structs ok to pass to swift?

can we use type_is_blittable ?

Blittable struct types should be allowed.

src/mono/mono/metadata/marshal.h

lambdageek · 2024-01-30T17:56:42Z

src/mono/mono/mini/mini-amd64.c

+	MonoClass *swift_error_ptr = mono_class_create_ptr (m_class_get_this_arg (swift_error));
+	for (guint i = 0; i < sig->param_count + sig->hasthis; i++) {
+		MonoClass *klass = mono_class_from_mono_type_internal (sig->params [i]);
+		if (klass && (klass == swift_error || klass == swift_error_ptr)) {


Why does this look for both SwiftError and SwiftError* ?

The idea is to support passing by reference using the ref keyword. However, it is not supported for function pointers, so it is necessary to support SwiftError* as well.

lambdageek · 2024-01-30T19:43:36Z

src/mono/mono/mini/mini-arm64.c

+				add_param (cinfo, ainfo, sig->params [pindex], FALSE);
+				ainfo->reg = ARMREG_R20;
+				continue;
+			} else if (klass == swift_error || klass == swift_error_ptr) {


same as x64 - why do we look for both SwiftError and SwiftError*

src/mono/mono/mini/mini.c

lambdageek · 2024-01-30T19:54:57Z

src/mono/mono/metadata/marshal.c


-	mspecs = g_new0 (MonoMarshalSpec*, sig->param_count + 1);
-	mono_method_get_marshal_info (method, mspecs);
+		if (mono_method_signature_has_ext_callconv (csig, MONO_EXT_CALLCONV_SWIFTCALL)) {


@kotlarmilos @vargaz Do we need to prevent these m2n swiftcall wrappers from being inlined? I think there's logic in the mini-arm64 and mini-amd64 backends that depends on the pinvoke bit of the signature.

Is the inlining performed before or after the lowering process? Do the method prolog and epilog get inlined as well?

ArgSwiftError is handled differently in the method prolog, which could be affected when it is inlined. Also, there is a potential problem related to context registers clobbering when wrappers are inlined.

These wrappers are not inlined right now.

Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>

BrzVlad · 2024-02-01T15:23:35Z

src/mono/mono/mini/mini-arm64.c

+			MonoClass *klass = mono_class_from_mono_type_internal (sig->params [pindex]);
+			if (klass == swift_self && sig->pinvoke) {
+				cinfo->gr--;
+				add_param (cinfo, ainfo, sig->params [pindex], FALSE);


I don't think we should call add_param in pinvoke cases. We know that the arg will always be passed in the specific register and I think we should initialize ainfo explicitly.

The idea was to reuse code and avoid maintaining changes in multiple places. However, as you suggested, it could be as simple as calculating the size of the payload.

BrzVlad · 2024-02-01T15:26:18Z

src/mono/mono/mini/mini-amd64.c

+				guint32 old_gr = gr;
+				if (gr >= PARAM_REGS)
+					gr--;
+				add_valuetype (sig, ainfo, ptype, FALSE, &gr, &fr, &stack_size);


This looks way more confusing that on arm64 case. We should probably just initialize ainfo explicitly as arg in ireg.

Ok, same as in arm64 case.

BrzVlad · 2024-02-01T15:47:20Z

src/mono/mono/mini/mini-amd64.c

@@ -1249,6 +1280,21 @@ mono_arch_set_native_call_context_args (CallContext *ccontext, gpointer frame, M
 		else
 			storage = arg_get_storage (ccontext, ainfo);

+#ifdef MONO_ARCH_HAVE_SWIFTCALL
+		if (mono_method_signature_has_ext_callconv (sig, MONO_EXT_CALLCONV_SWIFTCALL)) {


Do we even need all this code ? Looks like for swift self, we should have something like arg in ireg with the context reg specified in ainfo->reg. So arg get storage should already handle it. For swift error similarly the storage is returned, why additional comparison is required ? I think all this code could just be: if (ainfo == ArgSwiftError) *(gpointer*)storage = 0;

You are right. The ctx args are already set properly and should be handled without additional logic. Thanks for the heads up!

BrzVlad · 2024-02-01T15:49:10Z

src/mono/mono/mini/mini-arm64.c

@@ -2010,6 +2034,21 @@ mono_arch_set_native_call_context_args (CallContext *ccontext, gpointer frame, M
 		else
 			storage = arg_get_storage (ccontext, ainfo);

+#ifdef MONO_ARCH_HAVE_SWIFTCALL
+		if (mono_method_signature_has_ext_callconv (sig, MONO_EXT_CALLCONV_SWIFTCALL)) {


As in amd64 case, I think all this code can be removed. Here we would need to make arg_get_storage correctly return the storage for context regs, since the CallContext doesn't contain all the registers as on amd64.

BrzVlad

LGTM overall

…ilos/runtime into poc/swift-interop-direct-pinvoke

kotlarmilos · 2024-02-06T09:22:41Z

@dotnet/jit-contrib Please take a look at the runtime tests before the merge.

amanasifkhalid · 2024-02-06T15:45:15Z

runtime tests LGTM, too

kotlarmilos added 30 commits September 13, 2023 15:41

Add README.md

5136aaf

Merge branch 'dotnet:main' into poc/swift-interop-direct-pinvoke

674a156

Update build steps

9fc9402

Add experiment templates as unit tests

d279a03

Update README.md

de27030

Update README.md

476a605

Update README.md

b1b69d3

Update README.md

a95aca8

Update README.md

6c8e477

Merge branch 'dotnet:main' into poc/swift-interop-direct-pinvoke

b06c840

Fix the MathLibrary.swift

7810e56

Update README.md

6d9d67f

Update README.md

87adeed

Add Swift ABI details

0716fda

Merge branch 'poc/swift-interop-direct-pinvoke' of https://github.com…

ab53067

…/kotlarmilos/runtime into poc/swift-interop-direct-pinvoke

Update README.md

675619f

Add tasks to the README.md

24a8a01

Script that exports and demangles Swift symbols

11d60cc

Update README.md

e8169f0

Merge branch 'dotnet:main' into poc/swift-interop-direct-pinvoke

8584918

Add custom attributes for Swift calling convention

279a594

Add runtime support for Swift error handling

acc15a0

Add sample test

5ea8755

Add Swift error handling on minijit arm64

7a69077

Update README.md

ebe4116

Merge branch 'dotnet:main' into poc/swift-interop-direct-pinvoke

0c40694

Add mini arm64 support for Swift self context

71d33f4

Remove redundant movx instruction

7bcf61f

Merge branch 'dotnet:main' into poc/swift-interop-direct-pinvoke

7d9f73d

Update Swift types according to the design document

5973ff5

[mini] Preserve Swift error register value during the P/Invoke call

a60f392

The Swift error argument is not present in the P/Invoke signature, but the context register value can be changed by Swift. Therefore, it must be preserved during the P/Invoke call by updating the used_int_regs.

build-analysis bot mentioned this pull request Jan 25, 2024

"We stopped hearing from agent Azure Pipelines 32. Verify the agent machine is running and has a healthy network connection" dotnet/dnceng#1886

Open

3 tasks

kotlarmilos added 2 commits January 26, 2024 10:39

Merge branch 'dotnet:main' into poc/swift-interop-direct-pinvoke

bd11b7b

[mono] Allow SwiftError*, ref SwiftError, and function pointers in Ca…

2b2b75e

…llConvSwift signature

kotlarmilos requested review from BrzVlad and vargaz January 29, 2024 10:38

kotlarmilos added 2 commits January 30, 2024 11:43

[mono] Remove unused code and fix lint

6e54b5b

[mono] Avoid g_assert for Swift ctx regs

426802e

build-analysis bot mentioned this pull request Jan 30, 2024

[browser][MT] Unhandled JS error #97546

Closed

lambdageek approved these changes Jan 30, 2024

View reviewed changes

kotlarmilos and others added 3 commits January 31, 2024 12:18

Update src/mono/mono/metadata/marshal.c

4e09785

Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>

Update src/mono/mono/metadata/marshal.h

89d211c

Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>

Update Swift runtime tests to use blittable types

d450fb1

build-analysis bot mentioned this pull request Jan 31, 2024

[browser][mt] RangeError: Invalid atomic access index #96545

Closed

BrzVlad reviewed Feb 1, 2024

View reviewed changes

[mono] Simplify register allocation and interp native call context

1a39a88

BrzVlad approved these changes Feb 2, 2024

View reviewed changes

matouskozak added 2 commits February 4, 2024 17:10

Merge branch 'poc/swift-interop-direct-pinvoke' of github.com:kotlarm…

c2e354f

…ilos/runtime into poc/swift-interop-direct-pinvoke

align ArgSwiftError on x64

1405a98

jakobbotsch approved these changes Feb 6, 2024

View reviewed changes

amanasifkhalid approved these changes Feb 6, 2024

View reviewed changes

kotlarmilos merged commit be8cb2b into dotnet:main Feb 6, 2024
163 checks passed

github-actions bot locked and limited conversation to collaborators Mar 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mono] Add runtime support for Swift calling convention on x64 and arm64 #94764

[mono] Add runtime support for Swift calling convention on x64 and arm64 #94764

kotlarmilos commented Nov 15, 2023 •

edited

Loading

matouskozak commented Jan 25, 2024

azure-pipelines bot commented Jan 25, 2024

lambdageek left a comment

lambdageek Jan 30, 2024

kotlarmilos Jan 31, 2024

lambdageek Jan 30, 2024

kotlarmilos Jan 31, 2024

lambdageek Jan 30, 2024

lambdageek Jan 30, 2024

kotlarmilos Jan 31, 2024

vargaz Feb 2, 2024

BrzVlad Feb 1, 2024

kotlarmilos Feb 2, 2024

BrzVlad Feb 1, 2024

kotlarmilos Feb 2, 2024

BrzVlad Feb 1, 2024

kotlarmilos Feb 2, 2024 •

edited

Loading

BrzVlad Feb 1, 2024

BrzVlad left a comment

kotlarmilos commented Feb 6, 2024

amanasifkhalid commented Feb 6, 2024

		} else if (!m_class_is_enumtype (param_klass) && !m_class_is_primitive (param_klass) && m_class_get_this_arg (param_klass)->type != MONO_TYPE_FNPTR) {
		swift_error_args = swift_self_args = 0;

[mono] Add runtime support for Swift calling convention on x64 and arm64 #94764

[mono] Add runtime support for Swift calling convention on x64 and arm64 #94764

Conversation

kotlarmilos commented Nov 15, 2023 • edited Loading

Description

matouskozak commented Jan 25, 2024

azure-pipelines bot commented Jan 25, 2024

lambdageek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kotlarmilos Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrzVlad left a comment

Choose a reason for hiding this comment

kotlarmilos commented Feb 6, 2024

amanasifkhalid commented Feb 6, 2024

kotlarmilos commented Nov 15, 2023 •

edited

Loading

kotlarmilos Feb 2, 2024 •

edited

Loading