<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Oct 28, 2011, at 3:18 AM, Дмитрий Соколов wrote:</div><blockquote type="cite"><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-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
<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.</div></div></blockquote><div><br></div>Well, somehow your patch gives us the wrong type, so I imagine that</div><div>some invariant is being broken. LLVM definitely will not add padding</div><div>between the first two elements in that type.</div><div><br></div><div><blockquote type="cite"><div class="gmail_quote"><div>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.</div></div></blockquote><br></div><div>I don't really care about how we print out layouts for debugging; I just</div><div>want all the data to have the right offset in the IR struct type we make.</div><div><br></div><div>John.</div></body></html>