<br><br><div class="gmail_quote">2011/10/28 John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Oct 28, 2011, at 1:13 AM, r4start wrote:<br>
> On 27/10/2011 23:31, John McCall wrote:<br>
</div><div><div></div><div class="h5">>> +  } else if (<br>
>> +    Types.getContext().getTargetInfo().getCXXABI() == CXXABI_Microsoft) {<br>
>> +    // In MS mode this code helps insert padding bytes before fields,<br>
>> +    // like MSVC does.<br>
>> +    CharUnits padding = fieldOffset - NextFieldOffset;<br>
>> +<br>
>> +    AppendBytes(padding);<br>
>><br>
>> This still shouldn't be necessary.  This routine just adds padding bytes<br>
>> to get us up to fieldOffset, which is determined by the ASTRecordLayout.<br>
>> If the alignment of the next IR type is high enough, we don't need explicit<br>
>> padding bytes, which is what the 'if' condition above this is testing.  This<br>
>> isn't any different in the MS ABI.<br>
>><br>
>> That said, it does look like there's a bug here, because we *always* need<br>
>> to add padding bytes if we're building a packed layout, and that can<br>
>> happen in MSVC;  see above.  So I think the condition should be:<br>
>><br>
>>   if (alignedNextFieldOffset<  fieldOffset ||<br>
>>       (Packed&&  fieldOffset != NextFieldOffset)) {<br>
> This condition doesn't work for class I from test.<br>
> We have such layout<br>
> struct I<br>
>  0 |   (I vftable pointer)<br>
>  8 |   (I vbtable pointer)<br>
> 16 |   double q<br>
> 24 |   class D (virtual base)<br>
> 24 |     (D vftable pointer)<br>
> 32 |     double a<br>
> and layout must be %struct.I = type { i32 (...)**, i32*, [4 x i8], double, %class.D }<br>
> But we will have %struct.I = type { i32 (...)**, i32*, double, %class.D } and Packed = false.<br>
<br>
</div></div>Neither of these types is correct;  both incorrectly place the<br>
vbtbl at offset 4.  It's just that the first type happens to create<br>
some extra padding in the wrong place, and that makes the<br>
total struct size right.<br>
<br>
The type should be this:<br>
  type { i32 (...)**, [4 x i8], i32*, double, %class.D }<br>
<br>
If you place the vbtbl at the right offset, you should find that<br>
everything else falls out correctly.<br>
<font color="#888888"><br></font></blockquote><div>As you can see in my patch I place vbptr on offset from RecordLayout.<br>So I expect that LLVM add padding between them.<br>If we want show layout like MSVC then type must be<br>
type { i32 (...)**, [4 x i8], i32*, [4 x i8], double, %class.D } .<br>But MSVC never shows padding between vfptr and vbptr.<br><font color="#888888"><br><br> - Dmitry.</font></div></div><br>