[PATCH] [ms-cxxabi] Emit linkonce complete dtors in TUs that need them

[Reid Kleckner]

  Thanks for the review.


================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:42
@@ -64,1 +41,3 @@
+    // The deleting dtor is installed in the vtable and deferred to the end of
+    // the TU.
   }
----------------
Timur Iskhodzhanov wrote:
> Reid Kleckner wrote:
> > Timur Iskhodzhanov wrote:
> > > I don't like that the CHECKs are so far away from the code.
> > > Is it possible to just add some extra DTORS line before the first one that was here?
> > > 
> > > e.g. add
> > > 
> > >   void last_function_in_TU() {}
> > > 
> > > at the end of the file?
> > I believe I got rid of DTORS because deferred code isn't emitted in source order anyway.  Now that we have more than one deferred code blob, they have to be in reverse source order, so you can't keep the CHECK with the source unless you start adding more FileCheck invocations.  Multiple FileCheck invocations are slow.
> Do you think "fast" is more important than "easy to understand" ?
Both are important.  I'm really unhappy with how slow the vbtables test case is due to it's linear number of FileCheck invocations, but I also prefer it the way it is.

Anyway, I'll put this back.

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:225
@@ +224,3 @@
+
+// Now try some virtual bases, where we need the complete dtor.
+
----------------
Timur Iskhodzhanov wrote:
> the vbase dtor?
All of clang uses the Itanium terminology, so I don't see a reason to use the confusing MSVC terminology.

================
Comment at: lib/CodeGen/CGCXX.cpp:286
@@ +285,3 @@
+  // XXX: We could do this for Itanium, but we should consult with John first.
+  if (getTarget().getCXXABI().isMicrosoft() && dtorType == Dtor_Complete &&
+      dtor->getParent()->getNumVBases() == 0)
----------------
Timur Iskhodzhanov wrote:
> Wanted to use hasLinkOnceDestructorVariants here?
I'd rather remove the duplicate interface in Basic and just use the CGCXXABI interface.

================
Comment at: include/clang/Basic/TargetCXXABI.h:157
@@ +156,3 @@
+  /// delegate towards the base destructor?
+  bool hasLinkOnceDestructorVariants() const {
+    return isMicrosoft();
----------------
Timur Iskhodzhanov wrote:
> Do you use it?
No, it's dead.

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:255
@@ +254,3 @@
+struct B : virtual A { ~B(); };
+struct C : virtual A { ~C(); };
+struct D : B, C { ~D(); };
----------------
Timur Iskhodzhanov wrote:
> Do you need B/C here?
> Can you have the same structs as you do in namespace del_vbase ?
Sure, I can reuse the struct decls for these three test cases.

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:260
@@ +259,3 @@
+// CHECK: define void @"\01?destroy_d_complete at dtors2@@YAXXZ"
+// CHECK: call x86_thiscallcc void @"\01??_DD at dtors2@@QAE at XZ"
+// CHECK: ret
----------------
Timur Iskhodzhanov wrote:
> should probably make sure the 2nd arg is zero?
Only the deleting dtors take those flag arguments.

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:293
@@ +292,3 @@
+// CHECK: define linkonce_odr x86_thiscallcc void @"\01??_DB at del_vbase@@QAE at XZ"
+// CHECK: call x86_thiscallcc void @"\01??1B at del_vbase@@QAE at XZ"
+// CHECK: ret
----------------
Timur Iskhodzhanov wrote:
> Check that ~A gets called?
Merged with other dtor.


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