[cfe-dev] CGRecordLayoutBuilder for MS ABI

[r4start]

On 02/11/2011 13:17, John McCall wrote:
> On Nov 2, 2011, at 1:32 AM, r4start wrote:
>> On 02/11/2011 10:12, John McCall wrote:
>>> I'm sorry if I've asked this before, but have you verified that this is the
>>> correct condition?  Specifically, does the ABI not re-use vbptrs from
>>> non-primary non-virtual bases?
>>>
>>> For example, consider this hierarchy:
>>>    class A { int a; };
>>>    class B { int b; virtual void foo(); };
>>>    class C : public virtual A { int c; };
>>>    class D { int d; };
>>>    class E : public B, public virtual D { int e; };
>>>
>>> Does class E ultimately have two vbptrs or one?
>> In this test E will be have one vbptr. But if class E was declared like this "class E : public C, public virtual D" my code will generate 2 vbptr.
>> I fix it.
> Heh.  Yes, that's the test I meant, sorry.  Thanks.
>
>>> I'm still not sure why this is necessary.  AppendField should already
>>> be adding padding to bring NextFieldOffset up to VBPtrOffset.  The
>>> only time it doesn't add that padding explicitly is when the ABI
>>> alignment of the field type being added would implicitly introduce it.
>> As you can see AppendField just does :
>> 1. Save field type in types;
>> 2. Renew NextField offset (NextField = fieldOffset + fieldSize).
> Ah, sorry, you're right.  Shows how rusty I am with this code.  Then
> the appropriate thing to do is to call
>    AppendPadding(fieldOffset, getTypeAlignment(vbptr));
> before calling AppendField.
>
> Also, AppendField should at least get an
>    assert(fieldOffset == NextFieldOffset);
> or better yet the fieldOffset argument should disappear completely.
>
>>> +  if (alignedNextFieldOffset<   fieldOffset ||
>>> +      (!Packed&&   (fieldOffset != NextFieldOffset))) {
>> class D {
>> public:
>>   virtual void b(){}
>>   double a;
>> };
>> struct I : public virtual D
>> {
>>   virtual ~I(){}
>>   double q;
>> };
>> For this example, we see that fieldOffset = 0x10 for q and alignedNextFieldOffset = 0x10, but NextFieldOffset = 0x0c.
>> Second condition wrong for MS ABI and useless for other ABI.
> It's not wrong.  Like I've said several times now, LLVM struct types
> have very specific layout rules that are more complicated than
> "one field starts immediately after the last".  The rules are based
> roughly on C, except greatly simplified, which means they're
> not always correct for the actual source-language struct.
> Sometimes the source-language struct has more padding than
> the IR rules would dictate;  if so, we have to add extra padding
> bytes manually.  Sometimes the source-language struct has
> *less* padding than the IR rules would dictate;  if so, we have to
> use packed IR structs even when the source-language struct
> doesn't have __attribute__((packed)) on it.
>
> So in your example, it's okay for there not to be a [4 x i8]
> between the vbptr and the double because the target data for
> Win32 knows that 'double' needs to be 8-byte aligned and so
> implicitly puts padding there.  Contrariwise, it's not okay for
> there not to be a [4 x i8] between the vfptr and the vbptr
> because the target data for Win32 only knows that pointers
> need to be 4-byte aligned, so it wouldn't implicitly put padding
> there.
>
> And if the AST layout told us something even crazier, like
> that the double has to be at offset 12 even though it has 8-byte
> alignment under this target data, then we'd have to give up
> and use a packed LLVM struct type to represent it.
>
> John.
>
Thanks for the clarification.
I will return Packed in condition,
I was baffled by that StructLayout adds 4 bytes alignment at the end.

  - Dmitry.