[PATCH] D17863: Swift Calling Convention: add swiftcc

[Tim Northover via llvm-commits]

t.p.northover added inline comments.

================
Comment at: lib/Target/X86/X86CallingConv.td:196-199
@@ +195,6 @@
+def RetCC_X86_64_Swift : CallingConv<[
+  // For integers, ECX, R8D can be used as extra return registers.
+  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]>>,
----------------
rjmccall wrote:
> t.p.northover wrote:
> > manmanren wrote:
> > > t.p.northover wrote:
> > > > rjmccall wrote:
> > > > > t.p.northover wrote:
> > > > > > manmanren wrote:
> > > > > > > t.p.northover wrote:
> > > > > > > > manmanren wrote:
> > > > > > > > > Hi Tim,
> > > > > > > > > 
> > > > > > > > > i1 type is promoted to i8 before reaching here. That is why we don't need to handle i1 here.
> > > > > > > > > This is what other RetCCs do, such as RetCC_X86_32_Fast, but I am not against making it explicit here.
> > > > > > > > Where does it get promoted? To me it looks like RetCC_X86_64 directly calls RetCC_X86_64_Swift, which doesn't handle it. It then gets forwarded to RetCC_X86Common which promotes to i8 and only uses AL, DL, CL.
> > > > > > > > 
> > > > > > > > I suspect you'll find that something like
> > > > > > > > 
> > > > > > > >     define [4 x i1] @foo() {
> > > > > > > >         %res = insertvalue [4 x i1] undef, i1 true, 3
> > > > > > > >         ret [4 x i1] %res
> > > > > > > >     }
> > > > > > > > 
> > > > > > > > does not do what you're expecting.
> > > > > > > Sorry that it took so long for me to get back on this!
> > > > > > > 
> > > > > > > It was promoted in TargetLoweringBase::computeRegisterProperties
> > > > > > >   // Inspect all of the ValueType's smaller than the largest integer
> > > > > > >   // register to see which ones need promotion.
> > > > > > > 
> > > > > > > TLI.getRegisterType(...) for i1 will return i8.
> > > > > > > 
> > > > > > Ah, I see. The i1 case is only needed for AVX-512 by the looks of it. Do you know if Swift has a position on that? I would expect the Linux crowd to want to try it out when the chips come along properly, even if Macs don't have them yet.
> > > > > Steve Canon and I agreed that the swift CC should be stable independent of AVX settings, so you shouldn't ever see vectors larger than 16 bytes in backend lowering.
> > > > Enabling AVX-512 makes i1 a legal type which affects the CC even for plain i1s (well, if they'd get passed in r8b). E.g. compiling my example with "-mcpu=skylake-avx512" changes the calling convention to indirect sret.
> > > Yes, enabling AVX512 will make i1 a legal type. I wonder how other calling conventions (RetCC_X86_64_HiPE, RetCC_X86_64_WebKit_JS, RetCC_X86_32_Fast) do not need to handle i1.
> > > 
> > > I will add this line
> > > CCIfType<[i1],  CCPromoteToType<i8>>,
> > > 
> > > 
> > At least WebKit and Fast have no concerns about ABI stability. Not sure about HiPE.
> That is true for the C calling convention; it is not true for swiftcc.  swiftcc will always treat a 512-bit vector as illegal and split it up.  If we have an ABI break in the future that allows us to assume the existence of AVX, we can easily revisit that decision; but I specifically did not want to use the unstable C rule for swift.
Which bit? I don't think anyone's mentioned passing actual vectors; this is an issue with scalar i1s being handled differently depending on whether AVX-512 is around or not.


http://reviews.llvm.org/D17863