[PATCH, PowerPC] ABI fixes / improvements for powerpc64-linux

[Ulrich Weigand]

Hal Finkel <hfinkel at anl.gov> wrote on 08.07.2014 22:01:10:

> Why are we restricting to power-of-two element numbers? <3 x float>
> will become a <4 x float> and end up in vector registers anyhow.

Hmm ... I guess we are free to make that choice; there is no way to
create a <3 x float> type with GCC, so LLVM can in fact define the
ABI for such types as we prefer.

My code treats those as "non-Altivec" and passes in GPRs.  But
I guess we can instead treat them as <4 x float> Altivec and pass
in vector registers.

This would probably mean that we don't need an isAltivecVectorType
routine, and just check for getTypeSize == 128.

> These two hunks are very similar, can they be refactored into a
> single function?

If we go for the size == 128 test, the hunks would simplify anyway
since then there would be no type that needs two registers.

> Please add some tests for non-power-of-two-number-of-elements types.

I actually have a <6 x short> test already in.  I've now added a
<3 x short> test as well.

> > Aggregates are passed using the "byval" mechanism, so in theory
> > either
> > Clang or LLVM could compute the correct alignment required for an
> > aggregate
> > parameter.  However, it seems preferable to do this in Clang, since
> > it has
> > the more complete type information, and Clang needs the information
> > anyway
> > in order to expand va_arg correctly.  So this is what the following
> > patch
> > does.  As a result, every byval parameter will now carry an explicit
> > "align" attribute in the IR created by Clang (which I understand is
> > recommended anyway?).
>
> Yes.
>
> LGTM. Please add some tests for non-power-of-two-number-of-elements
types.

OK.  Added a test for a three-int struct.  Also added a test for a struct
with alignment 32, to verify it still only gets byval align 16 (see below).
Any further tests you'd like to see?

> +    // We may need to skip a slot to ensure alignment.
> +    if (isAlignedParamType(Ty) && (AllocatedSlots & 1))
> +      AllocatedSlots++;
>
> How do you know it will be only one? What happens if the type has an
> enhanced alignment (given by __attribute__((aligned(128))) for example).

Well, according to the ABI, no parameter is aligned at more than 16 bytes
in the *parameter area*, even if the type has a larger alignment
requirement.
That's why there will be at most one slot to skip.  (In fact, that's one of
the reasons of that part of the ABI -- we don't want to waste too many
opportunities to put values in registers.)

(Note that the prior patch also only supports 8 / 16 bytes of alignment
in the parameter save area.)

Attached updated patch set:

(See attached file: diff-clang-synthvector)
(See attached file: diff-clang-structalign)
(See attached file: diff-clang-structpass)

Bye,
Ulrich
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-clang-synthvector
Type: application/octet-stream
Size: 3628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140709/7213b806/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-clang-structalign
Type: application/octet-stream
Size: 8478 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140709/7213b806/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-clang-structpass
Type: application/octet-stream
Size: 8995 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140709/7213b806/attachment-0002.obj>