[llvm] r190636 - Fix PPC ABI for ByVal structs with vector members

[Hal Finkel]

----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "David Blaikie" <dblaikie at gmail.com>, llvm-commits at cs.uiuc.edu
> Sent: Friday, February 27, 2015 6:14:41 PM
> Subject: Re: [llvm] r190636 - Fix PPC ABI for ByVal structs with vector members
> 
> Hal Finkel <hfinkel at anl.gov> wrote on 28.02.2015 00:41:39:
> 
> > and so we always get the alignment correct, because it is the
> > alignment if 'i128' (which is 16). To digress slightly, I'm now
> > somewhat concerned that this works more-or-less by accident
> > (because
> > DataLayout only defaults to giving natural alignment to vector
> > types, for integers we normally just take the largest integer
> > alignment specified, which would be 8 in this case, and for arrays
> > we take the alignment of the element type) but all of the
> > temporaries have 16 byte alignment, and so it happens to do the
> > right thing (Uli implemented this, so I'm cc'ing him here).
> 
> So the way I intended this to work is that clang specifies the
> expected alignment in the argument save area by the *size* of
> the array element type (i.e. aggregates that require 8-byte
> alignment are passed as arrays of i64, and aggregates that
> require 16-byte alignment as passed as arrays of i128).
> 
> In LLVM we respect that request by looking at the *size* of the
> array element type in CalculateStackSlotAlignment:
> 
>   // Array members are always packed to their original alignment.
>   if (Flags.isInConsecutiveRegs()) {
>     // If the array member was split into multiple registers, the
>     first
>     // needs to be aligned to the size of the full type.  (Except for
>     // ppcf128, which is only aligned as its f64 components.)
>     if (Flags.isSplit() && OrigVT != MVT::ppcf128)
>       Align = OrigVT.getStoreSize();
>     else
>       Align = ArgVT.getStoreSize();
>   }
> 
> The important bit is the "OrigVT.getStoreSize" here, which will
> be 128 for an array of i128.
> 
> This may be a bit nonintuitive, but it was the only way I found to
> pass alignment information from clang to LLVM if I didn't want to
> use ByVal.

Ah, that's what I was missing; thanks!

> 
> I did extensive testing (in particular, using GCC's foreign compiler
> ABI test suite to make sure GCC and LLVM are interoperable), so I
> don't think it works just by accident :-)

You're indeed correct. Thanks for clarifying that.

 -Hal

> 
> Bye,
> Ulrich
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory