<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rnk<br class="gmail_msg">
Date: Mon Sep 12 19:01:23 2016<br class="gmail_msg">
New Revision: 281278<br class="gmail_msg">
<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=281278&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=281278&view=rev</a><br class="gmail_msg">
Log:<br class="gmail_msg">
[DebugInfo] Deduplicate debug info limiting logic<br class="gmail_msg">
<br class="gmail_msg">
We should be doing the same checks when a type is completed as we do<br class="gmail_msg">
when a complete type is used during emission. Previously, we duplicated<br class="gmail_msg">
the logic, and it got out of sync. This could be observed with<br class="gmail_msg">
dllimported classes.<br class="gmail_msg"></blockquote><div><br>Thanks for having a go at this!<br><br>The logic in completeType (& maybe also completeClassData) still seems to duplicate some of the logic in shouldOmitDefinition - I think the original idea I had was a series of cascading entry points (completeType checking the "oh, the type now has a definition", then falling into completeRequiredType, etc)<br><br>In theory the "shouldOmitDefinition" could be split into the extra conditions for each of the parts of that cascade, so the checks wouldn't be redundantly executed when we were already being called about the completion or required completion of a type, etc. But I'm not sure the performance gain would be measurable, etc.<br><br>If not, we might as well just have one entry point that's something like "revisit type because something interesting happened to it" (it became complete, it became required to be complete, etc)... but that seems a bit weird too.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
Also reduce a test case for this slightly.<br class="gmail_msg"></blockquote><div><br>The UseCompleteType is still there/unnecessary (as is the 'private' in UnicodeString) - and I'd probably rename it to foo/bar and throw it in a namespace like the rest for consistency - having semantic names in a test when they're just remnants from whatever code it was reduced from seems confusing to me. But I realize I'm a bit more pedantic about that than most.<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
Implementing review feedback from David Blaikie on r281057.<br class="gmail_msg">
<br class="gmail_msg">
Modified:<br class="gmail_msg">
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br class="gmail_msg">
cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp<br class="gmail_msg">
cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp<br class="gmail_msg">
<br class="gmail_msg">
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281278&r1=281277&r2=281278&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281278&r1=281277&r2=281278&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br class="gmail_msg">
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Sep 12 19:01:23 2016<br class="gmail_msg">
@@ -1644,27 +1644,6 @@ void CGDebugInfo::completeType(const Rec<br class="gmail_msg">
completeRequiredType(RD);<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
-void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {<br class="gmail_msg">
- if (DebugKind <= codegenoptions::DebugLineTablesOnly)<br class="gmail_msg">
- return;<br class="gmail_msg">
-<br class="gmail_msg">
- // If this is a dynamic class and we're emitting limited debug info, wait<br class="gmail_msg">
- // until the vtable is emitted to complete the class debug info.<br class="gmail_msg">
- if (DebugKind <= codegenoptions::LimitedDebugInfo) {<br class="gmail_msg">
- if (const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD))<br class="gmail_msg">
- if (CXXDecl->isDynamicClass())<br class="gmail_msg">
- return;<br class="gmail_msg">
- }<br class="gmail_msg">
-<br class="gmail_msg">
- if (DebugTypeExtRefs && RD->isFromASTFile())<br class="gmail_msg">
- return;<br class="gmail_msg">
-<br class="gmail_msg">
- QualType Ty = CGM.getContext().getRecordType(RD);<br class="gmail_msg">
- llvm::DIType *T = getTypeOrNull(Ty);<br class="gmail_msg">
- if (T && T->isForwardDecl())<br class="gmail_msg">
- completeClassData(RD);<br class="gmail_msg">
-}<br class="gmail_msg">
-<br class="gmail_msg">
void CGDebugInfo::completeClassData(const RecordDecl *RD) {<br class="gmail_msg">
if (DebugKind <= codegenoptions::DebugLineTablesOnly)<br class="gmail_msg">
return;<br class="gmail_msg">
@@ -1763,6 +1742,16 @@ static bool shouldOmitDefinition(codegen<br class="gmail_msg">
return false;<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
+void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {<br class="gmail_msg">
+ if (shouldOmitDefinition(DebugKind, DebugTypeExtRefs, RD, CGM.getLangOpts()))<br class="gmail_msg">
+ return;<br class="gmail_msg">
+<br class="gmail_msg">
+ QualType Ty = CGM.getContext().getRecordType(RD);<br class="gmail_msg">
+ llvm::DIType *T = getTypeOrNull(Ty);<br class="gmail_msg">
+ if (T && T->isForwardDecl())<br class="gmail_msg">
+ completeClassData(RD);<br class="gmail_msg">
+}<br class="gmail_msg">
+<br class="gmail_msg">
llvm::DIType *CGDebugInfo::CreateType(const RecordType *Ty) {<br class="gmail_msg">
RecordDecl *RD = Ty->getDecl();<br class="gmail_msg">
llvm::DIType *T = cast_or_null<llvm::DIType>(getTypeOrNull(QualType(Ty, 0)));<br class="gmail_msg">
<br class="gmail_msg">
Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281278&r1=281277&r2=281278&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281278&r1=281277&r2=281278&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp (original)<br class="gmail_msg">
+++ cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp Mon Sep 12 19:01:23 2016<br class="gmail_msg">
@@ -6,10 +6,7 @@<br class="gmail_msg">
// more general than that.<br class="gmail_msg">
<br class="gmail_msg">
struct UnicodeString;<br class="gmail_msg">
-struct GetFwdDecl {<br class="gmail_msg">
- static UnicodeString format;<br class="gmail_msg">
-};<br class="gmail_msg">
-GetFwdDecl force_fwd_decl;<br class="gmail_msg">
+UnicodeString *force_fwd_decl;<br class="gmail_msg">
struct UnicodeString {<br class="gmail_msg">
private:<br class="gmail_msg">
virtual ~UnicodeString();<br class="gmail_msg">
<br class="gmail_msg">
Modified: cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp?rev=281278&r1=281277&r2=281278&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp?rev=281278&r1=281277&r2=281278&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp (original)<br class="gmail_msg">
+++ cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp Mon Sep 12 19:01:23 2016<br class="gmail_msg">
@@ -4,6 +4,10 @@<br class="gmail_msg">
// be imported from a DLL. Otherwise, the debugger wouldn't be able to show the<br class="gmail_msg">
// members.<br class="gmail_msg">
<br class="gmail_msg">
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "ImportedAfterCompletion",<br class="gmail_msg">
+// CHECK-NOT: DIFlagFwdDecl<br class="gmail_msg">
+// CHECK-SAME: ){{$}}<br class="gmail_msg">
+<br class="gmail_msg">
// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "OutOfLineCtor",<br class="gmail_msg">
// CHECK-SAME: DIFlagFwdDecl<br class="gmail_msg">
// CHECK-SAME: ){{$}}<br class="gmail_msg">
@@ -16,6 +20,13 @@<br class="gmail_msg">
// CHECK-NOT: DIFlagFwdDecl<br class="gmail_msg">
// CHECK-SAME: ){{$}}<br class="gmail_msg">
<br class="gmail_msg">
+<br class="gmail_msg">
+struct ImportedAfterCompletion;<br class="gmail_msg">
+ImportedAfterCompletion *force_fwd_decl;<br class="gmail_msg">
+struct __declspec(dllimport) ImportedAfterCompletion {<br class="gmail_msg">
+ virtual ~ImportedAfterCompletion();<br class="gmail_msg">
+};<br class="gmail_msg">
+<br class="gmail_msg">
struct OutOfLineCtor {<br class="gmail_msg">
OutOfLineCtor();<br class="gmail_msg">
virtual void Foo();<br class="gmail_msg">
@@ -35,6 +46,7 @@ struct ImportedMethod {<br class="gmail_msg">
};<br class="gmail_msg">
<br class="gmail_msg">
int main() {<br class="gmail_msg">
+ ImportedAfterCompletion c;<br class="gmail_msg">
OutOfLineCtor o;<br class="gmail_msg">
DerivedFromImported d;<br class="gmail_msg">
ImportedMethod m;<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
cfe-commits mailing list<br class="gmail_msg">
<a href="mailto:cfe-commits@lists.llvm.org" class="gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class="gmail_msg">
</blockquote></div></div>