<div dir="ltr">I think what Eli said makes sense.<div><br></div><div>I'd add that maybe the concept of "preferred alignment" is closer to what you want.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Apr 24, 2020 at 12:55 PM Eli Friedman via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">





<div lang="EN-US">
<div class="gmail-m_-2392059176231077802WordSection1">
<p class="MsoNormal">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?<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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”.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Also, probably worth checking the layout for a testcase like the following, to check the interaction between the extra padding and the nvsize:<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">class A { double a; int b; };<u></u><u></u></p>
<p class="MsoNormal">class B : A { int c; };<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">-Eli<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal" style="margin-left:0.5in"><b>From:</b> cfe-dev <<a href="mailto:cfe-dev-bounces@lists.llvm.org" target="_blank">cfe-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>Xiangling Liao via cfe-dev<br>
<b>Sent:</b> Friday, April 24, 2020 6:21 AM<br>
<b>To:</b> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<b>Subject:</b> [EXT] [cfe-dev] [RFC] Adding AIX power alignment rule in clang front end<u></u><u></u></p>
</div>
<p class="MsoNormal" style="margin-left:0.5in"><u></u> <u></u></p>
<div>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">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.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">This rule is recursive. It applies all the way to the inner most nested members. An example is as the following:<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-right:0in;margin-bottom:3pt;margin-left:0.5in">
<span style="font-size:9pt;font-family:"Courier New";color:black">struct D {<br>
        double d;<br>
        int i;<br>
};</span><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-right:0in;margin-bottom:3pt;margin-left:0.5in">
<span style="font-size:9pt;font-family:"Courier New";color:black"> </span><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-right:0in;margin-bottom:3pt;margin-left:0.5in">
<span style="font-size:9pt;font-family:"Courier New";color:black">struct IntFirst {<br>
        int i;<br>
        struct D d;<br>
};</span><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif;color:black"> </span><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif;color:black">The size of struct D is 16, class alignment is 8;</span><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif;color:black">The size of struct IntFirst is 20, class alignment is 4;</span><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">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.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">And based on this idea, there are two possible ways.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">1.
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">The first one is that we introduced a new field in class
<i>ASTRecordLayout</i><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">Called `<i>AIXOffsetAlignment</i>`  which represents the special AIX alignment for the object that<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">contains floating-point member or sub-member. This is for AIX-ABI only.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">The proposed change has been put on Phabricator under review:
<a href="https://reviews.llvm.org/D78563" target="_blank"><span style="color:rgb(5,99,193)">https://reviews.llvm.org/D78563</span></a>
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><b><span style="font-size:12pt;font-family:"Times New Roman",serif">Pros:</span></b><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">- The design is cleaner. It is clear that such field is for AIX and it does not affect any other target
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">   in terms of functionality.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-right:0in;margin-bottom:12pt;margin-left:0.5in;text-align:justify">
<span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u> <u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><b><span style="font-size:12pt;font-family:"Times New Roman",serif">Cons:</span></b><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">- All other targets using ItaniumRecordLayoutBuilder will also have to pay the price to keep
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">   updating this `<i>AIXOffsetAlignment`
</i>along the common code path by using <i>`UpdateAlignment</i>`. <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">2.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">The second possible way we have in mind is to overload the usage of current Microsoft-ABI only alignment field
<i>`RequiredAlignment</i>` and rename it to `<i>TargetSpecialAlignment</i>` instead of the above first way to create a new alignment field.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">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.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><b><span style="font-size:12pt;font-family:"Times New Roman",serif">Pro:</span></b><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">- Class ASTRecordLayout does not need to construct one more field `AIXOffsetAlignment` compared to the method one.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-right:0in;margin-bottom:12pt;margin-left:0.5in">
<span style="font-size:12pt;font-family:"Times New Roman",serif">- 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.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:39pt;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:39pt;text-align:justify"><b><span style="font-size:12pt;font-family:"Times New Roman",serif">Cons:</span></b><span style="font-size:12pt;font-family:"Times New Roman",serif"><u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">-  Still, all other targets using ItaniumRecordLayoutBuilder will also have to pay the price to   <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">   keep updating this `TargetSpecialAlignment` along the common code path by using
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">   `UpdateAlignment`.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">-  By overloading the usage, it may create confusion when more targets try to utilize it in the
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">   future.
<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in"><span style="font-size:12pt;font-family:"Times New Roman",serif">-  The degree of willingness of sharing this current MS-only alignment field is unknown.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">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?<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">Please let me know if there are any questions about my current proposal and design. Your feedback is appreciated.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif">Regards,<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif;color:rgb(136,136,136)"> <u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:0.5in;text-align:justify"><span style="font-size:12pt;font-family:"Times New Roman",serif;color:rgb(136,136,136)">Xiangling Liao<u></u><u></u></span></p>
</div>
</div>
</div>

_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>