[PATCH] D17863: Swift Calling Convention: add swiftcc

[Tim Northover via llvm-commits]

t.p.northover requested changes to this revision.
t.p.northover added a comment.
This revision now requires changes to proceed.

Hi Manman,

I think the testing is too weak if we're going for ABI stability (particularly based on FastCC, which is explicitly called out in the LangRef as unstable and based on APCS). I'd actually strongly suggest Swift starts out with AAPCS_VFP on ARM.

Even with that, I'd suggest testing things like minimum stack slot size, alignment of objects on the stack, whether everything goes on the stack if something has. Probably on both iOS and WatchOS because I suspect those details differ based on type alignment.

For x86, you don't seem to test the new xmm registers at all.

Cheers.

Tim.


================
Comment at: lib/Target/X86/X86CallingConv.td:196-199
@@ +195,6 @@
+def RetCC_X86_64_Swift : CallingConv<[
+  CCIfType<[i8] , CCAssignToReg<[AL, DL, CL, R8B]>>,
+  CCIfType<[i16], CCAssignToReg<[AX, DX, CX, R8W]>>,
+  CCIfType<[i32], CCAssignToReg<[EAX, EDX, ECX, R8D]>>,
+  CCIfType<[i64], CCAssignToReg<[RAX, RDX, RCX, R8]>>,
+
----------------
Why did you drop the `CCIfType<[i1],  CCPromoteToType<i8>>`? Does Swift never return anything containing an i1 or something?

Probably worth commenting on the differences here too (adding R8, XMM2 and XMM3 it seems).

================
Comment at: test/CodeGen/ARM/swift-ios.ll:8-13
@@ +7,8 @@
+; CHECK: vadd.f32
+  %a.addr = alloca float, align 4
+  %b.addr = alloca float, align 4
+  store float %a, float* %a.addr, align 4
+  store float %b, float* %b.addr, align 4
+  %0 = load float, float* %a.addr, align 4
+  %1 = load float, float* %b.addr, align 4
+  %add = fadd float %0, %1
----------------
I don't think this is needed. Same with the other tests.


http://reviews.llvm.org/D17863