General comment: is there code we actually try to compile with Clang that
really needs vector deleting dtors support?
Are these dtors needed anywhere other than polymorphic array delete?
IMO what we need in the MS ABI dtors handling at this stage is general
correctness of when common types of destructors get called,
that is roughly:
# ?1X@@... to destroy an object of type X without inheritance/vftables
# ?1Base@@... to destroy a non-virtual base class Base from ?1Child@@...
# //Any// of the deleting destructors for a polymorphic non-array delete
# The scalar deleting dtor of "struct Child : virtual Base" calls the
virtual base dtor (?_DChild@@...).
Currently we're pretty much at 1+2+3 if we just skip emitting the base dtors
(similar to how we skip emitting base ctors).
That said, I like some of the refactoring you've done in this patch, but I'm
worried that the progress on vector deleting dtors was done at the cost of
degrading the existing functionality.
What do you think about working on the virtual base dtors first, writing more
tests (so we understand the dtors ABI better) and then taking a new look at the
vector deleting dtors?
================
Comment at: lib/CodeGen/CodeGenModule.cpp:1223
@@ -1208,2 +1222,3 @@
return true;
if (CodeGenOpts.OptimizationLevel == 0 &&
+ !GD.getDecl()->hasAttr<AlwaysInlineAttr>() &&
----------------
how about
const FunctionDecl *F = GD.getDecl();
before the second if?
================
Comment at: test/CodeGenCXX/microsoft-abi-static-initializers.cpp:14
@@ -13,3 +13,3 @@
// CHECK: define internal void @"__dtor_\01?s@@3US@@A"() [[NUW]] {
-// CHECK: call x86_thiscallcc void @"\01??1S@@QAE@XZ"
+// CHECK: call x86_thiscallcc void @"\01??_DS@@QAE@XZ"
// CHECK: ret void
----------------
Should be "?1S@@...", otherwise you might get errors when linking TU compiled
with different compilers, right?
================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:69
@@ +68,3 @@
+// DTOR3: %[[FLAGS_VALUE:[0-9a-z._]+]] = load i32* %[[FLAGS_VAR]]
+// DTOR3: call x86_thiscallcc void
@"\01??_DC@basic@@UAE@XZ"(%"struct.basic::C"* %[[THIS:[0-9a-z]+]])
+// DTOR3-NEXT: %[[CONDITION:[0-9]+]] = icmp eq i32 %[[FLAGS_VALUE]], 0
----------------
Should be "?1C@..." here.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:277
@@ -273,3 +276,3 @@
} else if (IsDeletingDtor(CGF.CurGD)) {
- ImplicitParamDecl *ShouldDelete
+ ImplicitParamDecl *Flags
= ImplicitParamDecl::Create(Context, 0,
----------------
Can you please document that the least significant bit is "whether need to call
operator delete at the end" and the second least significant bit is "whether a
polymorphic array delete" ?
http://llvm-reviews.chandlerc.com/D746
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
