[PATCH] Abstract out the emission of vtables, add basic support for vtable emission when using -cxx-abi microsoft

Timur Iskhodzhanov timurrrr at google.com
Fri Sep 27 07:51:32 PDT 2013


  Thanks!


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:661
@@ +660,3 @@
+  llvm::Value *VTableAddressPoint =
+      CGM.getCXXABI().getAddrOfVTable(VTableClass, Base.getBaseOffset());
+  if (!VTableAddressPoint) {
----------------
Reid Kleckner wrote:
> No need for "CGM.getCXXABI()" inside MicrosoftCXXABI.
Probably a Ctrl+X - Ctrl-V mistake...
Thanks for catching!

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:663
@@ +662,3 @@
+  if (!VTableAddressPoint) {
+    const CXXRecordDecl *BaseDecl = Base.getBase();
+    assert(BaseDecl->getNumVBases() &&
----------------
Reid Kleckner wrote:
> This will warn in a clang NDEBUG build, probably just fold it into the assert.
Done

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:694
@@ +693,3 @@
+
+  llvm::GlobalVariable *&VTable = VFTablesMap[ID];
+
----------------
Reid Kleckner wrote:
> You can avoid the double DenseMap lookup with .insert(), and you can use llvm:tie() to avoid the ugly std::pair type:
> 
>   VFTablesMapTy::iterator I;
>   bool Inserted;
>   llvm::tie(I, Inserted) = VFTablesMap.insert(0);
>   ...
Wow!

It turned out not to be as good though due to make_pair vs 0 vs llvm::GV*, but I agree it's at least more effective.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:707
@@ +706,3 @@
+    // a unique mangled name.
+    llvm::StringSet<> ObservedMangledNames;
+    for (size_t J = 0, F = VFPtrs.size(); J != F; ++J) {
----------------
Reid Kleckner wrote:
> Put this in #ifndef NDEBUG.
Done


http://llvm-reviews.chandlerc.com/D1532



More information about the cfe-commits mailing list