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

[Timur Iskhodzhanov]

  LGTM with a few nits/minor questions


================
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.
   }
----------------
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" ?

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

================
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)
----------------
Wanted to use hasLinkOnceDestructorVariants here?

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:225
@@ +224,3 @@
+
+// Now try some virtual bases, where we need the complete dtor.
+
----------------
the vbase dtor?

================
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(); };
----------------
Do you need B/C here?
Can you have the same structs as you do in namespace del_vbase ?

================
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
----------------
should probably make sure the 2nd arg is zero?

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:286
@@ +285,3 @@
+// CHECK: define void @"\01?call_deleting_dtor at del_vbase@@YAXPAUB at 1@@Z"
+// CHECK: call x86_thiscallcc void @"\01??_DB at del_vbase@@QAE at XZ"
+// CHECK: ret
----------------
should probably make sure the 2nd arg is one?

================
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
----------------
Check that ~A gets called?


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