Skip to content

JIT: Add support for struct returns from Swift reverse pinvokes - #100091

Merged
jakobbotsch merged 37 commits into
dotnet:mainfrom
jakobbotsch:swift-callback-struct-returns
Mar 23, 2024
Merged

JIT: Add support for struct returns from Swift reverse pinvokes#100091
jakobbotsch merged 37 commits into
dotnet:mainfrom
jakobbotsch:swift-callback-struct-returns

Conversation

@jakobbotsch

@jakobbotschjakobbotsch commented Mar 21, 2024

Copy link
Copy Markdown
Member

Update the 10 tests to have some struct returns as well.

We also have to change the prestub to save rax since it's used as a register for the ret buffer in Swift calls.

@jakobbotsch
Add test
9a9a052
@jakobbotsch
Support SwiftSelf
bfd4e28
@jakobbotsch
Backend handling
53d00a6
@jakobbotsch
Fix arm64 build
ea5c172
@jakobbotsch
Add missing handling of DNER reason
4fd5964
@jakobbotsch
Fix assert
ea32187
@jakobbotsch
Fix build
e01d190
@jakobbotsch
Remove some unnecessarily aggressive asserts
40d998a
@jakobbotsch
Expand assert
4c64049
@jakobbotsch
Reduce tests
7d43f3c
@jakobbotsch
Disable tests on Mono
49826a8
@jakobbotsch
Regenerate tests
d65c4c3
@jakobbotsch
Factor int parameter register validation
3e76803
@jakobbotsch
Add missing license headers
21d1c52
@jakobbotsch
More feedback
b4b73d7
@jakobbotsch
Run jit-format
cb44f0b
@jakobbotsch
Support fixed ret buff on x64 with Swift CC
1d25a57
@jakobbotsch
Run jit-format
df7558f
@jakobbotsch
Save rax in pre stub
5e72f8d
@jakobbotsch
Fix TheUMEntryPrestub
97d8b03
@jakobbotsch
Revert "Fix TheUMEntryPrestub"
20c9fec
This reverts commit 97d8b03.
@jakobbotsch
Revert "Save rax in pre stub"
291368c
This reverts commit 5e72f8d.
@jakobbotsch
Alternative
4394228
@jakobbotsch
Use proper offsets
88996be
@jakobbotsch
Add some jitdump
06883e3
@jakobbotsch
Move logging and ensure it's under verbose
8b2aef7
@jakobbotsch
Run jit-format
12fd147
@jakobbotsch
Fix build
9de8490
@jakobbotsch
New tests
be1a948
@jakobbotsch
Merge branch 'main' of github.com:dotnet/runtime into swift-callback-…
a973085
…struct-returns
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 21, 2024
@jakobbotschjakobbotsch mentioned this pull request Mar 21, 2024
Closed
This was referenced Mar 22, 2024
Open
Closed
@jakobbotsch

jakobbotsch commented Mar 22, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr jitstress, runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

@azure-pipelines

azure-pipelinesBot commented Mar 22, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 22, 2024 12:37
@jakobbotsch

jakobbotsch commented Mar 22, 2024

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @amanasifkhalid

Failures look like #99810 and other known failures

FYI @jkurdek@kotlarmilos@matouskozak

amanasifkhalid
amanasifkhalid approved these changes Mar 22, 2024
View reviewed changes

@amanasifkhalidamanasifkhalid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Comment threadsrc/coreclr/jit/lclvars.cpp
{
printf("Swift compilation returns %s as %d primitive(s) in registers\n",
typGetObjLayout(retTypeHnd)->GetClassName(), lowering->numLoweredElements);
for (size_t i = 0; i < lowering->numLoweredElements; i++)

@amanasifkhalidamanasifkhalidMar 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we dump these lowered element types in a few places now, right? You may want to consider moving this logic to a helper method (though feel free to do that in a future PR).

@jakobbotschjakobbotschMar 22, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Makes sense to factor this, let me do that in a follow-up.

@build-analysisbuild-analysisBot mentioned this pull request Mar 22, 2024
Closed
@jakobbotsch

jakobbotsch commented Mar 22, 2024

Copy link
Copy Markdown
MemberAuthor

Also FYI @jkoritzinsky and cc @jkotas for the stub change.

jkotas
jkotas reviewed Mar 22, 2024
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/theprestubamd64.S Outdated
jkotas
jkotas reviewed Mar 22, 2024
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/theprestubamd64.S Outdated
jakobbotschand others added 2 commits March 22, 2024 20:59
@jakobbotsch@jkotas
Update src/coreclr/vm/amd64/theprestubamd64.S
844be3c
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@jakobbotsch@jkotas
Update src/coreclr/vm/amd64/theprestubamd64.S
cfc4aef
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@jkotas

jkotas commented Mar 22, 2024

Copy link
Copy Markdown
Member

The stub change LGTM

This was referenced Mar 23, 2024
Closed
Closed
@jakobbotsch

jakobbotsch commented Mar 23, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime

@azure-pipelines

azure-pipelinesBot commented Mar 23, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
jakobbotsch merged commit d981ed9 into dotnet:mainMar 23, 2024
@jakobbotsch
jakobbotsch deleted the swift-callback-struct-returns branch March 23, 2024 19:15
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 23, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotasjkotasjkotas left review comments
+1 more reviewer
@amanasifkhalidamanasifkhalidamanasifkhalid approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jakobbotschjakobbotsch

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@jkotas@amanasifkhalid