================
Comment at: lib/AST/VTableBuilder.cpp:2723
@@ -2720,1 +2722,3 @@
 
+    // If a "Base" class has at least one non-virtual base with a virtual
+    // destructor, the "Base" virtual destructor will take the address of the
----------------
maybe say "at least one path to a non-virtual base with..."

================
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) {
----------------
Timur Iskhodzhanov wrote:
> 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
Oh, that's surprising.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:586
@@ +585,3 @@
+    bool AvoidVirtualOffset = false;
+    if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base) {
+      // A base destructor can only be called from a complete destructor of the
----------------
At this point more than half of the method is specific to destructors.  Maybe 
it would be cleaner to duplicate the ML lookup and split this into an 
adjustThisArgumentForVirtualDtorCall, and explain why we *have* to use the 
static offset in some cases?


http://llvm-reviews.chandlerc.com/D1939
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to