[PATCH] Fix vbtable indices if a class shares the vbptr with a non-virtual base

Timur Iskhodzhanov timurrrr at google.com
Sun Nov 3 09:33:01 PST 2013


  Forgot to put the word "distinction" in the previous comment.

  Please take another look at the patch!


================
Comment at: lib/AST/VTableBuilder.cpp:2415-2416
@@ +2414,4 @@
+              BaseOffset = Layout.getBaseClassOffset(CurBase);
+    if (DerivedVBPtrOffset < BaseOffset)
+      continue;
+    const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(CurBase);
----------------
Reid Kleckner wrote:
> Can this happen?  I thought we always just used the first nvbase with vbases.  If so, I'd make that the loop exit criteria, since it's simpler than this vbptr offset comparison.
OK, I've removed this microoptimization and after some thought replaced "continue" below with "break".

================
Comment at: lib/CodeGen/MicrosoftVBTables.cpp:213
@@ -213,2 +212,3 @@
     Offset -= VBPtrSubobject.getBaseOffset() + VBPtrOffset;
-    Offsets.push_back(llvm::ConstantInt::get(CGM.IntTy, Offset.getQuantity()));
+    llvm::Constant *&VBOffset = Offsets[GetVBTableIndex(ReusingBase, VBase)];
+    assert(VBOffset == 0 && "The same vbindex seen twice?");
----------------
Reid Kleckner wrote:
> This is silly, we're walking the inheritance graph in a loop.  Make something that returns the full BaseSetVectorTy, and we can use that to fill in this table.
OK, I left the loop in place, but the getVBTableIndex results are now cached inside MicrosoftVTableContext. I don't think adding an extra interface function and an extra RD-to-Vector map is worth it for just one function using them.


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



More information about the cfe-commits mailing list