Move VTableContext & friends from AST to CodeGen?

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?

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

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,

I have no information that I can share on PathScale’s current activities. I have copied in Christopher Bergstrom who may be able to provide more information.

I would say though that the process of moving the vtable builder to AST was a huge pain and it would be good if nobody else would have to go through it.

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.)

I agree that there's really no reason to pretend that there's a common interface here; we should have an ItaniumVTableContext and a MicrosoftVTableContext. However, please maintain the current design where the abstract layout for the vftbl and vbtbl are generated in the AST library and then consumed by IR-generation.

John.

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.)

I agree that there's really no reason to pretend that there's a common interface here; we should have an ItaniumVTableContext and a MicrosoftVTableContext.

Well, looking at VTableContext I actually did have a feeling that the
interface is small enough to generalize.
But on the second thought - yeah, I think the VTableContexts should be
different too (not only VTableBuilders).

This way, we'll have to move the BuildVirtualCall into CGCXXABI (as
the exact this adjustments, vptr offsets and vtable indices are
different in different ABIs),
so the abstraction happens in CGCXXABI rather than VTableContext.
I think I'll try to get round CGVTables then and just use
MicrosoftVTableContext directly in the CodeGen's MicrosoftCXXABI.

However, please maintain the current design where the abstract layout for the vftbl and vbtbl are generated in the AST library and then consumed by IR-generation.

OK, will do.

Thanks!