<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Jul 22, 2013, at 4:10 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br><div><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">2013/7/20 John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>>:<br><blockquote type="cite">On Jul 19, 2013, at 8:33 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br><br>2013/7/19 John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>>:<br><br>+ void LayoutVFTable() {<br><br>+ // FIXME: unclear what to do with RTTI in MS ABI as emitting it<br>anywhere<br>+ // breaks the vftable layout. Just skip RTTI for now, can't mangle<br>anyway.<br>+<br><br>Can you explain how it breaks the vftable layout? My understanding is that<br>disabling RTTI actually changes the ABI by causing the RTTI pointer to<br>be skipped in the layout, thus changing all the offsets; is it more than<br>that?<br><br><br>Please note this comment is copied from<br>VTableBuilder::LayoutPrimaryAndSecondaryVTables.<br><br>As far as I understand, if we emit RTTI, it should go to vftable[-1].<br>Otherwise there's even no slot allocated for it.<br>That means the vftable symbol should point in the middle of a section<br>(if RTTI is emitted), which is not yet supported by LLVM IIRC (see the<br>"Symbol in the middle of a global" thread).<br><br><br>Ah, gotcha. Yes, having LLVM and linker support for this would be great.<br><br>I think we should also use weak/strong-odr linkage for no-RTTI/RTTI<br>cases so the more complete vftables are chosen at link-time.<br><br><br>I thought vf-tables never had strong linkage?<br></blockquote><br>Sorry, I've used a slightly wrong term.<br>The vftables are COMDAT sections with the "select the largest<br>definition if different sizes are available" flag.<br></div></blockquote><div><br></div>Interesting. Yeah, that deserves to have its own linkage specifier in LLVM IR.</div><div><br><blockquote type="cite" dir="auto"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite">+ // See if this class expands a vftable of one of its bases.<br>+ const CXXRecordDecl *NextBase = 0, *NextLastVBase = LastVBase;<br><br>Why isn't this always just the primary base? In what situations do<br>virtual function entries ever get added to a vf-table from a non-primary<br>base?<br><br><br>consider<br>struct A;<br>struct B;<br>struct C: B;<br>struct MDC: A, C;<br>if C adds new methods to B, they are put into the B's vftable.<br>Basically, each class in the hierarchy can add methods to its own<br>primary base vftable.<br><br><br>Right, but from the perspective of C, they’re going in its primary vf-table<br>(which is the same as B-in-C’s primary vf-table).<br><br>This is why I was saying (1) identify tables by the most derived base<br>subobject for which they’re the primary vf-table and then (2) recurse<br>down the primary base chain from there (adding methods as you<br>move back towards the most derived base subobject).<br></blockquote><br>Ah I see.<br>The thing is that I've been clearing the BasePath in case there's only<br>one vftable in a class.<br>I've removed this code now, deferring the decision to the mangler.<br><br><blockquote type="cite">You have a lot of very hard-to-follow path logic in this code; it's very<br>difficult to review this without understanding the invariants better.<br><br><br>I had the same complaint while reading VTableBuilder/VTableContext :)<br>That said, I'll be happy to put explaining comments into the code<br>where necessary, but it's already hard for me to decide what needs<br>more comments...<br><br>It seems like MethodInfoMap is basically trying to track the current<br>class's notion of the final overriders of all the methods in its primary<br>vf-table?<br><br><br>Not in the primary vftable but rather in the vftable it's building<br>(e.g. first vbase's votable).<br><br><br>By "current class”, I meant the class for which you are currently adding<br>methods.<br><br>Other than that - yes.<br>I think my MethodInfoMap is similar to MethodInfoMap from<br>VTableBuilder::AddMethods.<br>Please note that using FinalOverriders alone would not suffice due to<br>a different "this parameter" logic in this ABI.<br><br><br>Right, but all that is is a mapping of methods to methods from a<br>different base subobject.<br></blockquote><br>Can you please clarify if I should add any comments for this into the code?<br></div></blockquote><div><br></div></div>I think putting a comment on the recursive method stating what you expect<div>to be in MethodInfoMap after each iteration would make a lot of sense.</div><div><br></div><div>John.</div></body></html>