[PATCH] [-cxx-abi microsoft] Fix "this" argument/parameter offsets for virtual destructors in the presence of virtual bases

[Timur Iskhodzhanov]

================
Comment at: lib/AST/VTableBuilder.cpp:2714
@@ +2713,3 @@
+
+        // The traversal ends at the vbase for virtual destructors.
+        if (isa<CXXDestructorDecl>(MD))
----------------
Reid Kleckner wrote:
> So, before calling a dtor, we adjust to the first virtual base with a virtual dtor.  Is that right?
Somewhat.
Please see the revised version of the comment and note this code is inside a for loop iterating over base paths.

Basically, if there's at least one non-virtual base with a virtual destructor or there are no vbases at all, the "this" argument should point to the MDC;
otherwise it points to the first vbase with a vdtor

================
Comment at: lib/AST/VTableBuilder.cpp:2722-2723
@@ -2720,1 +2721,4 @@
 
+    // If there's at least one virtual destructor in a non-virtual base,
+    // the "this" parameter points at the most derived class.
+    if (!SeenVBase && isa<CXXDestructorDecl>(MD))
----------------
Reid Kleckner wrote:
> Wouldn't you return zero if it points at the most derived class, or was the old code wrong?
The old code was wrong. See a revised comment.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:575
@@ +574,3 @@
+    // There's only Dtor_Deleting in vftable but it shares the this adjustment
+    // with the deleting one, so look it up instead.
+    LookupGD = GlobalDecl(DD, Dtor_Deleting);
----------------
Reid Kleckner wrote:
> I don't understand this comment.  Do you mean that the base dtor should use the same this adjustment as the deleting one?
Oops! Fixed the comment

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:585
@@ -574,5 +584,3 @@
   if (ML.VBase) {
-    This = CGF.Builder.CreateBitCast(This, charPtrTy);
-    llvm::Value *VBaseOffset = CGM.getCXXABI()
-        .GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
-    This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
+    bool AvoidVirtualOffset = false;
+    if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base) {
----------------
Reid Kleckner wrote:
> I'm guessing this optimization just makes it easier to test and read and test the LLVM IR output.
Err, no. We replace a dynamic vbtable lookup with a static adjustment, which is not only faster but can also lead to different results!

Interestingly, the result of the static adjustment will be correct, not the result of dynamic adjustment - see the code with "This one is interesting" comment in VTableBuilder.cpp


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