This is an archive of the discontinued LLVM Phabricator instance.

Swift Calling Convention: add swiftself attribute
ClosedPublic

Authored by manmanren on Mar 3 2016, 1:54 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

manmanren updated this revision to Diff 49774.Mar 3 2016, 1:54 PM
manmanren retitled this revision from to Swift Calling Convention: add swiftself attribute.
manmanren updated this object.
manmanren added reviewers: lhames, rengolin, rnk.
manmanren added subscribers: llvm-commits, rjmccall.
rnk edited edge metadata.Mar 10 2016, 3:26 PM

Is this the special "context" argument that John mentioned? R10 is not a CSR on x86_64.

lib/Target/AArch64/AArch64CallingConvention.td
129 ↗(On Diff #49774)

"A SwitfSelf" maybe?

lib/Target/ARM/ARMCallingConv.td
26 ↗(On Diff #49774)

ditto, and elsewhere

test/CodeGen/X86/swiftself.ll
1 ↗(On Diff #49774)

It seems you have no special rules for passing a 32-bit swiftself. That makes sense, I don't imagine you care about x86_32 swift performance. Can you add a RUN line that shows it being spilled to the stack as usual, though? I assume you'll want the ABI for it to be stable.

Hi Reid,

Thanks for reviewing!

In D17866#372537, @rnk wrote:

Is this the special "context" argument that John mentioned? R10 is not a CSR on x86_64.

Yes, this is the "context/self" argument. This patch does not actually use a CSR because of the complications in the backend to use a CSR to pass an argument.
We are trying to improve this by improving the backend to handle this.

Cheers,
Manman

lib/Target/AArch64/AArch64CallingConvention.td
129 ↗(On Diff #49774)

Will do :]

test/CodeGen/X86/swiftself.ll
1 ↗(On Diff #49774)

You are right. I will make sure it works as designed on 32-bit.

manmanren updated this revision to Diff 50382.Mar 10 2016, 5:12 PM
manmanren edited edge metadata.
manmanren updated this revision to Diff 50383.Mar 10 2016, 5:15 PM

LGTM.

Reid, did you have anything else?

mehdi_amini added inline comments.Mar 29 2016, 9:51 AM
include/llvm/Target/TargetCallingConv.h
87 ↗(On Diff #50383)

Typo here: Flags |= ...

rnk accepted this revision.Mar 29 2016, 9:55 AM
rnk edited edge metadata.

Yep, looks good.

test/Bitcode/swiftself.ll
1 ↗(On Diff #50383)

Can you merge this into attributes.ll? Each new test file has a small cost, and this fits in with the existing test there.

This revision is now accepted and ready to land.Mar 29 2016, 9:55 AM
This revision was automatically updated to reflect the committed changes.
MatzeB added inline comments.Mar 29 2016, 6:22 PM
llvm/trunk/test/CodeGen/AArch64/swiftself.ll
1–2

CHECK-APPLE and CHECK-O0 appear to be checking the same why not just use CHECK? The same situations applies to the other tests.