[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