[cfe-dev] Move VTableContext & friends from AST to CodeGen?

[Peter Collingbourne]

Hi Timur,

My opinion is that the VTableContext-level interfaces for the two ABIs
(were they to exist) should be two separate interfaces without a base
class, since the details are fundamentally different at that level, and
that they can later be unified at the CodeGen level.  Since I'm guessing
that you don't really need a VTableContext equivalent if you're only
interested in LLVM IR generation, you could consider implementing Microsoft
vtable stuff wholly within CodeGen, while keeping the Itanium VTableContext
in AST.  (With sufficient interest, someone could factor the relevant parts
of the Microsoft vtable builder to AST as I did for Itanium.)


On Wed, May 15, 2013 at 3:31 PM, Timur Iskhodzhanov <timurrrr at google.com>wrote:

> Anders,
> Thanks for the historic insight!
>
> Looks like here are the discussion and the commit:
> http://comments.gmane.org/gmane.comp.compilers.clang.scm/36181
> http://llvm.org/viewvc/llvm-project?view=revision&revision=140510
>
> Peter,
> Do you know if PathScale is still working on a WHIRL backend for clang?
>   (I believe this was the main rationale for the move to AST)
> What exactly do they need to have in AST?
> Do they only support one C++ ABI?
>
> --
> Timur
>
> 2013/5/15 Anders Carlsson <andersca at icloud.com>:
> > Hi Timur,
> >
> > VTableBuilder actually started its life inside CodeGen, but was moved to
> AST because someone wanted to be able to use the clang fronted with a
> non-LLVM backend.
> >
> > I forget the exact details, but I'm pretty sure you can dig through svn
> or mailing list history and find out.
> >
> > - Anders
> >
> > On May 15, 2013, at 5:32 AM, Timur Iskhodzhanov <timurrrr at google.com>
> wrote:
> >
> >> Hi Anders, John,
> >>
> >> Is there any reason for AST/VTableBuilder.{h,cpp} to be in AST?
> >>
> >> I think these classes are only used in CodeGen.
> >> Here is an approximate list of files that use VTableContext,
> >> VTableLayout and VTableComponent:
> >>  CGClass.cpp
> >>  CGCXX.cpp
> >>  CGCXXABI.cpp
> >>  CGDebugInfo.cpp
> >>  CGExprConstant.cpp
> >>  CGExprCXX.cpp
> >>  CGRTTI.cpp
> >>  CGVTables.cpp
> >>  CGVTT.cpp
> >>  CodeGenModule.h
> >>
> >> The reason I'm asking is because the current code layout makes it
> >> harder to add support for Microsoft ABI vftables.
> >>
> >> Instead of having a single vtable with address points for non-primary
> >> bases (like in Itanium ABI), in Microsoft ABI we need to use a
> >> separate vftable builder for each base with a vfptr.
> >> Thus, we need to define these tables (in CGVTables?) and also emit
> >> stores to vfptrs in class constructors (in CGF?). We should be able to
> >> ask the VTableContext "hey, can you give me a list of virtual tables
> >> I need to define; and what vfptr I should emit stores for".
> >>
> >> As you can see, the code for dealing with vtables in ABI-specific way
> >> is currently split
> >> over at least three places, thus making it much harder to add
> >> ABI-specific functionality.
> >> I've just looked at Reid's VBTables patch where he just wrote a
> >> VBTableBuilder in the middle of CGVTables and it's so much simpler to
> >> connect with CodeGen than what I'm doing in VTableBuilder.h for
> >> vftables...
> >>
> >> I believe this should all be available through just one interface
> >> (CGVTables?) which should have different implementations like
> >> ItaniumABIVTables (with VTT) and MicrosoftABIVTables (with VBTables)
> >> to be used from ItaniumCXXABI / MicrosoftCXXABI respectively.
> >> This way, we'll consolidate the V{,F,B}Tables-related code from at
> >> least three different places to just one.
> >> This interface should also provide methods deal with virtual calls as
> >> they slightly differ in these ABIs as well and tied to the vtables.
> >> What do you think about this general direction?
> >>
> >> Would you mind if I move AST/VTableBuilder into CodeGen as the first
> step?
> >>
> >> --
> >> Timur Iskhodzhanov,
> >> Google Russia.
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130515/a49f71a2/attachment.html>