Patch to force SuitableAlign's alignment when loading on object with larger alignment

[Reid Kleckner]

Also, I should put some discussion I had with David in email.

I think maybe TypeInfo should be a triple of Width, Align, and
RequiredAlignment (or AttributedAlign or some other name). If we want to go
hog wild, we can encode it as log2 in two bytes to keep the struct small.
Right now the MSVC record layout algorithm queries getTypeInfo twice: once
with the fully desugared type and once with the regular type so it can get
both alignments. It uses one or the other depending on the presence of
pragma pack attributes.

This information could also help Itanium record layout of structs with
ms_struct attributes.


On Fri, Aug 1, 2014 at 11:44 AM, Reid Kleckner <rnk at google.com> wrote:

> What is HasAlignAttribute, and how is it different from
> AlignmentIsRequired? I have a feeling that they are different names for the
> same thing. If you want to rename it, that's fine with me.
>
>
> On Fri, Aug 1, 2014 at 10:45 AM, jahanian <fjahanian at apple.com> wrote:
>
>> Hi John,
>>
>> Here is the revised patch.
>>
>>
>> - Fariborz
>>
>> On Jul 28, 2014, at 6:58 PM, John McCall <rjmccall at apple.com> wrote:
>>
>> On Jul 28, 2014, at 4:22 PM, jahanian <fjahanian at apple.com> wrote:
>>
>> This patch enforces SuitableAlign’s alignment when loading objects with
>> more relaxed alignment. Currently, SuitableAlign is used in a
>> warning when type is over aligned. This patch optionally enforces this in
>> IRGen. I defined a new field in TargetInfo which is optionally set to
>> SuitableAlign. Currently, this is only defined for Apple’s targets as I
>> don’t know its implication for all other supported targets. Long term,
>> we want to use one value for both (Sema and code gen). This is rdar://
>> 16254558
>>
>>
>> We can’t change the ABI alignment; that would change struct layout.
>>
>> The right way of doing this is a code generation option that caps the
>> effective alignment that we’ll honor on an access to a pointer lacking an
>> explicit alignment attribute.  It should be exposed to users; I suggest
>> spelling it something like -fmax-type-align=N, with an option to disable it
>> completely called -fno-max-type-align.  You can have the driver pass down
>> -fmax-type-align=16 by default when targeting Darwin.
>>
>> +  static bool RestrictedCompleteObjectAlign(QualType T) {
>>
>> What this is really asking is whether the type has an explicit alignment
>> attribute.  The most reasonable thing to do is to have
>> ASTContext::getTypeInfoImpl compute (and cache) this information; that will
>> correctly look through recursive sugar, which you are not doing.  You can
>> add a new accessor (ASTContext::getTypeAlignInCharsAndIsExplicit?) which
>> returns a pair of the alignment and this bool.
>>
>> You should add a test case that checks that a typedef of an aligned
>> typedef is still considered explicitly aligned.
>>
>> There is nothing here that should be specific to ElaboratedType.
>>  Whatever that logic is, it should be testing for RecordType.  But I
>> actually think you should be applying the same logic to structs and unions
>> that you do to any other type: we should be capping the (non-ABI) alignment
>> of structs and unions unless they have an explicit AlignedAttr.
>>
>> John.
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140801/bbb39296/attachment.html>