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