[PATCH] Initialize vtorDisp in class constructors and destructors

[Reid Kleckner]

  LGTM

  These are nits, IMO this is the right interface.  There's no reason to push InitializeVTablePointers into CGCXXABI.  We initialize the same set of vtable pointers for Itanium and Microsoft.


================
Comment at: lib/CodeGen/CGCXXABI.h:240
@@ +239,3 @@
+  virtual void
+  initializeHiddenVirtualInheritanceMembers(CodeGenFunction &CGF,
+                                            const CXXRecordDecl *RD) {}
----------------
OK, this can't be part of EmitCtorCompleteObjectHandler.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:464-469
@@ +463,8 @@
+    CodeGenFunction &CGF, const CXXRecordDecl *RD) {
+  // In most cases, an override for a vbase virtual method can adjust
+  // the "this" parameter by applying a constant offset.
+  // However, this is not always enough when we execute a constructor or a
+  // destructor of some class X with virtual bases,
+  // X overrides a virtual method M of a vbase Y and
+  // X itself is a vbase of the most derived class.
+  // vtorDisp for vbase Y is a hidden member of X which holds the extra amount
----------------
nit: this text is oddly ragged.  Maybe the two conditions should be bulletted?

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:484-489
@@ +483,8 @@
+
+    llvm::Value *VBaseOffset = CGM.getCXXABI().GetVirtualBaseClassOffset(
+        CGF, getThisValue(CGF), RD, I->first);
+    uint64_t ConstantVBaseOffset =
+        Layout.getVBaseClassOffset(I->first).getQuantity();
+    llvm::Value *VtorDispValue = Builder.CreateSub(
+        VBaseOffset, llvm::ConstantInt::get(CGM.IntPtrTy, ConstantVBaseOffset));
+
----------------
Can you write a pseudo-code comment for this?  Something like:
// this->vtordisp_i = this->vbptr[vbase_idx] - offsetof(RD, VBase);

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:491-494
@@ +490,6 @@
+
+    unsigned AS = cast<llvm::PointerType>(getThisValue(CGF)->getType())
+                      ->getAddressSpace();
+    llvm::Value *Int8This =
+        Builder.CreateBitCast(getThisValue(CGF), CGF.Int8Ty->getPointerTo(AS));
+    llvm::Value *VtorDispPtr = Builder.CreateInBoundsGEP(Int8This, VBaseOffset);
----------------
Hoist this out of the loop for cleaner IR.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:500
@@ +499,3 @@
+        VtorDispPtr, CGF.Int32Ty->getPointerTo(AS), "vtordisp.ptr");
+    VtorDispValue = Builder.CreateTruncOrBitCast(VtorDispValue, CGF.Int32Ty,
+                                                 "vtordisp.val");
----------------
Do we need this?  We can use Int32Ty above instead of IntTy if you're worried about mismatch.


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