LGTM. I applied it in r202088.<br><br><div>On Fri Feb 21 2014 at 9:22:35 AM, Stephan Tolksdorf <<a href="mailto:st@quanttec.com">st@quanttec.com</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 14-02-12 22:21, Stephan Tolksdorf wrote:<br>
> On 14-02-12 20:45, David Majnemer wrote:<br>
>> This looks like the right place to fix the bug.<br>
>><br>
>> Three comments:<br>
>> - The '*' for pointer types should be on the right side.<br>
>> - AlignedAttr::getMaxAlignment() isn't always super fast but I guess<br>
>> such typedefs are rare.  An argument could be made that getTypeInfoImpl<br>
>> should return a struct containing the size, alignment and whether or not<br>
>> the type has an explicit alignment.  Then, getPreferredTypeAlign() could<br>
>> take advantage of it.<br>
>> - Both of your tests in CodeGen/alignment.c already pass without your<br>
>> patch.<br>
><br>
> Thanks a lot for the review!<br>
><br>
> I've moved the '*' to the right and split off the alignment.c changes<br>
> into a separate patch.<br>
><br>
> I suppose that typedefs with attributes are so rare that the overhead of<br>
> the new getMaxAlignment call should be minimal.<br>
><br>
> Please commit the patches if you think they are ready. (I don't have<br>
> commit access.)<br>
<br>
Ping.<br>
<br>
If you're concerned about the performance, I could also move the<br>
getMaxAlignment call into the 'if (T == double || T == long long ||<br>
...)' statement below and only call getMaxAlignment if ABIAlign !=<br>
getTypeSize(T).<br>
<br>
- Stephan<br>
<br>
</blockquote>