[cfe-dev] CGRecordLayoutBuilder for MS ABI

[John McCall]

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.