Parse CallConvSwift in CoreCLR/NativeAOT #96707
Conversation
ghost
assigned 
src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs
Outdated
Show resolved
Hide resolved
jkoritzinsky
force-pushed
the
callconv-swift-coreclr
branch
from
5e0f80a
to
ebe34e0
Compare
…he calling convention isn't in the method signature)
src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs
Outdated
Show resolved
Hide resolved
…yuJIT and the type systems will handle lowering
…ime into callconv-swift-coreclr
jkoritzinsky
changed the title
src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Swift/SwiftTypes.cs
Show resolved
Hide resolved
| @@ -130,6 +130,18 @@ public static bool IsMarshallingRequired(MethodDesc targetMethod) | |||
| return false; | |||
| } | |||
|
|
|||
| public static bool IsMarshallingRequired(MethodSignature methodSig, ModuleDesc moduleContext, UnmanagedCallingConventions callingConvention) | |||
There was a problem hiding this comment.
The callingConvention parameter is not used, is it intentional?
Why do we need a new overload though? I would expect the existing IsMarshallingRequired to get an extra callingConvention argument if answer to this now depends on the calling convention as well (or why can we still decide it without the calling convention in the other overload?).
There was a problem hiding this comment.
In the other overload, we can extract the calling convention from the MethodSignature. This overload is for UnmanagedCallersOnly methods where the callconv is in the attribute, not the managed signature.
There was a problem hiding this comment.
The parameter is still unused, is that intentional?
There was a problem hiding this comment.
Yes, it's intentional that it's unused.
src/coreclr/tools/Common/TypeSystem/Interop/UnmanagedCallingConventions.cs
Show resolved
Hide resolved
… ensure that the JIT/VM bail out on Swift callconv functions as of now.
Diff results for #96707Throughput diffsThroughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.00% to +0.01%)
Details here Throughput diffs for linux/x64 ran on linux/x64MinOpts (-0.01% to 0.00%)
Details here |
src/coreclr/tools/Common/TypeSystem/Interop/UnmanagedCallingConventions.cs
Outdated
Show resolved
Hide resolved
Diff results for #96707Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Throughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Details here Throughput diffs for linux/x64 ran on linux/x64MinOpts (-0.01% to -0.00%)
Details here |
| @@ -135,6 +135,25 @@ public static UnmanagedCallingConventions GetPInvokeMethodCallingConventions(thi | |||
| return result; | |||
| } | |||
|
|
|||
| public static UnmanagedCallingConventions GetDelegateCallingConventions(this TypeDesc delegateType) | |||
There was a problem hiding this comment.
This method doesn't appear used. Is that intentional?
There was a problem hiding this comment.
I removed the usage of it during a refactor. I think it's still useful to have, but we can remove it and re-add later when we have a use case.
| { | ||
| Debug.Assert(delegateType.IsDelegate); | ||
|
|
||
| if (delegateType is EcmaType ecmaDelegate) |
There was a problem hiding this comment.
Nit: Might want to either do delegateType.GetTypeDefinition() is EcmaType ecmaDelegate or assert the type is not generic so that we just don't do bad codegen if we ever allow interop with generic delegates.
| @@ -130,6 +130,18 @@ public static bool IsMarshallingRequired(MethodDesc targetMethod) | |||
| return false; | |||
| } | |||
|
|
|||
| public static bool IsMarshallingRequired(MethodSignature methodSig, ModuleDesc moduleContext, UnmanagedCallingConventions callingConvention) | |||
There was a problem hiding this comment.
The parameter is still unused, is that intentional?
Add support for recognizing CallConvSwift in CoreCLR and the managed type system to pass to the JIT. Mark the
Swift*types as intrinsics.Also, improve some rough edges around CallConv parsing in the managed type system around UnmanagedCallersOnly APIs