[cfe-dev] [RFC] Adding AIX power alignment rule in clang front end

[Reid Kleckner via cfe-dev]

I think what Eli said makes sense.

I'd add that maybe the concept of "preferred alignment" is closer to what
you want.

On Fri, Apr 24, 2020 at 12:55 PM Eli Friedman via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> Assuming I’m understanding the rule correctly, it doesn’t actually affect
> “alignment” of the type in the sense we would normally understand it.  A
> struct with a double as the first member can be stored at an address with
> four-byte alignment.  The rule only involves inserting extra padding at the
> end of the struct. In particular, from the way you’re describing the rule,
> I think the special alignment rule isn’t supposed to affect the result of
> _Alignof.  Does that seem right?
>
>
>
> Given that, do you actually need to store anything in the RecordLayout at
> all?  You could just write a check like “if the first member of the struct
> is a double (or recursively, a struct where the first member is a double),
> round up the size of the struct to a multiple of 8”.
>
>
>
> Also, probably worth checking the layout for a testcase like the
> following, to check the interaction between the extra padding and the
> nvsize:
>
>
>
> class A { double a; int b; };
>
> class B : A { int c; };
>
>
>
> -Eli
>
>
>
> *From:* cfe-dev <cfe-dev-bounces at lists.llvm.org> *On Behalf Of *Xiangling
> Liao via cfe-dev
> *Sent:* Friday, April 24, 2020 6:21 AM
> *To:* cfe-dev at lists.llvm.org
> *Subject:* [EXT] [cfe-dev] [RFC] Adding AIX power alignment rule in clang
> front end
>
>
>
> Currently we are trying to implement AIX special alignment rule on the top
> of Itanium ABI in Clang. On AIX,  in aggregates, the first member of
> double/long double is aligned according to its natural alignment value;
> subsequent double/long double of the aggregate are aligned on 4-byte
> boundaries.
>
>
>
> This rule is recursive. It applies all the way to the inner most nested
> members. An example is as the following:
>
>
>
> struct D {
>         double d;
>         int i;
> };
>
>
>
> struct IntFirst {
>         int i;
>         struct D d;
> };
>
>
>
> The size of struct D is 16, class alignment is 8;
>
> The size of struct IntFirst is 20, class alignment is 4;
>
>
>
> So our current idea is to have an alignment field to record nested struct
> info and pass the special alignment value all the way back to the ourter
> most aggregate.
>
>
>
> And based on this idea, there are two possible ways.
>
>
>
> 1.
>
> The first one is that we introduced a new field in class *ASTRecordLayout*
>
> Called `*AIXOffsetAlignment*`  which represents the special AIX alignment
> for the object that
>
> contains floating-point member or sub-member. This is for AIX-ABI only.
>
>
>
> The proposed change has been put on Phabricator under review:
> https://reviews.llvm.org/D78563
>
>
>
> *Pros:*
>
> - The design is cleaner. It is clear that such field is for AIX and it
> does not affect any other target
>
>    in terms of functionality.
>
>
>
> *Cons:*
>
> - All other targets using ItaniumRecordLayoutBuilder will also have to pay
> the price to keep
>
>    updating this `*AIXOffsetAlignment` *along the common code path by
> using *`UpdateAlignment*`.
>
>
>
> 2.
>
> The second possible way we have in mind is to overload the usage of
> current Microsoft-ABI only alignment field *`RequiredAlignment*` and
> rename it to `*TargetSpecialAlignment*` instead of the above first way to
> create a new alignment field.
>
> This alignment field will function the same in Itanium ABI part for AIX.
> Meanwhile, because the current `RequiredAlignment` is only used in MS code,
> renaming will not affect MS code path functionally.
>
>
>
> *Pro:*
>
> - Class ASTRecordLayout does not need to construct one more field
> `AIXOffsetAlignment` compared to the method one.
>
> - Instead of having each target add new fields for their special alignment
> rules, we could have one field that handles all the potential alignment
> differences for every target.
>
>
>
> *Cons:*
>
> -  Still, all other targets using ItaniumRecordLayoutBuilder will also
> have to pay the price to
>
>    keep updating this `TargetSpecialAlignment` along the common code path
> by using
>
>    `UpdateAlignment`.
>
> -  By overloading the usage, it may create confusion when more targets try
> to utilize it in the
>
>    future.
>
> -  The degree of willingness of sharing this current MS-only alignment
> field is unknown.
>
>
>
>
>
> I would like to see how people think about the above two ways. Which one
> is better? Any concerns about either one? Or any other ideas to suggest?
>
>
>
> Please let me know if there are any questions about my current proposal
> and design. Your feedback is appreciated.
>
>
>
> Regards,
>
>
>
> Xiangling Liao
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200424/aeba04e8/attachment-0001.html>