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@dtors2@@YAXXZ"
+// CHECK: call x86_thiscallcc void @"\01??_DD@dtors2@@QAE@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@del_vbase@@YAXPAUB@1@@Z"
+// CHECK: call x86_thiscallcc void @"\01??_DB@del_vbase@@QAE@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@del_vbase@@QAE@XZ"
+// CHECK: call x86_thiscallcc void @"\01??1B@del_vbase@@QAE@XZ"
+// CHECK: ret
----------------
Check that ~A gets called?
http://llvm-reviews.chandlerc.com/D1066
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
