r281278 - [DebugInfo] Deduplicate debug info limiting logic
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 19 10:00:27 PDT 2016
ping
On Mon, Sep 12, 2016 at 5:20 PM David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> Author: rnk
> Date: Mon Sep 12 19:01:23 2016
> New Revision: 281278
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281278&view=rev
> Log:
> [DebugInfo] Deduplicate debug info limiting logic
>
> We should be doing the same checks when a type is completed as we do
> when a complete type is used during emission. Previously, we duplicated
> the logic, and it got out of sync. This could be observed with
> dllimported classes.
>
>
> Thanks for having a go at this!
>
> 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)
>
> 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.
>
> 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.
>
>
>
> Also reduce a test case for this slightly.
>
>
> 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.
>
> - Dave
>
>
>
> Implementing review feedback from David Blaikie on r281057.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
> cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281278&r1=281277&r2=281278&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Sep 12 19:01:23 2016
> @@ -1644,27 +1644,6 @@ void CGDebugInfo::completeType(const Rec
> completeRequiredType(RD);
> }
>
> -void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
> - if (DebugKind <= codegenoptions::DebugLineTablesOnly)
> - return;
> -
> - // If this is a dynamic class and we're emitting limited debug info,
> wait
> - // until the vtable is emitted to complete the class debug info.
> - if (DebugKind <= codegenoptions::LimitedDebugInfo) {
> - if (const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD))
> - if (CXXDecl->isDynamicClass())
> - return;
> - }
> -
> - if (DebugTypeExtRefs && RD->isFromASTFile())
> - return;
> -
> - QualType Ty = CGM.getContext().getRecordType(RD);
> - llvm::DIType *T = getTypeOrNull(Ty);
> - if (T && T->isForwardDecl())
> - completeClassData(RD);
> -}
> -
> void CGDebugInfo::completeClassData(const RecordDecl *RD) {
> if (DebugKind <= codegenoptions::DebugLineTablesOnly)
> return;
> @@ -1763,6 +1742,16 @@ static bool shouldOmitDefinition(codegen
> return false;
> }
>
> +void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
> + if (shouldOmitDefinition(DebugKind, DebugTypeExtRefs, RD,
> CGM.getLangOpts()))
> + return;
> +
> + QualType Ty = CGM.getContext().getRecordType(RD);
> + llvm::DIType *T = getTypeOrNull(Ty);
> + if (T && T->isForwardDecl())
> + completeClassData(RD);
> +}
> +
> llvm::DIType *CGDebugInfo::CreateType(const RecordType *Ty) {
> RecordDecl *RD = Ty->getDecl();
> llvm::DIType *T = cast_or_null<llvm::DIType>(getTypeOrNull(QualType(Ty,
> 0)));
>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281278&r1=281277&r2=281278&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp Mon Sep 12
> 19:01:23 2016
> @@ -6,10 +6,7 @@
> // more general than that.
>
> struct UnicodeString;
> -struct GetFwdDecl {
> - static UnicodeString format;
> -};
> -GetFwdDecl force_fwd_decl;
> +UnicodeString *force_fwd_decl;
> struct UnicodeString {
> private:
> virtual ~UnicodeString();
>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp?rev=281278&r1=281277&r2=281278&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp
> (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp Mon Sep
> 12 19:01:23 2016
> @@ -4,6 +4,10 @@
> // be imported from a DLL. Otherwise, the debugger wouldn't be able to
> show the
> // members.
>
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name:
> "ImportedAfterCompletion",
> +// CHECK-NOT: DIFlagFwdDecl
> +// CHECK-SAME: ){{$}}
> +
> // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name:
> "OutOfLineCtor",
> // CHECK-SAME: DIFlagFwdDecl
> // CHECK-SAME: ){{$}}
> @@ -16,6 +20,13 @@
> // CHECK-NOT: DIFlagFwdDecl
> // CHECK-SAME: ){{$}}
>
> +
> +struct ImportedAfterCompletion;
> +ImportedAfterCompletion *force_fwd_decl;
> +struct __declspec(dllimport) ImportedAfterCompletion {
> + virtual ~ImportedAfterCompletion();
> +};
> +
> struct OutOfLineCtor {
> OutOfLineCtor();
> virtual void Foo();
> @@ -35,6 +46,7 @@ struct ImportedMethod {
> };
>
> int main() {
> + ImportedAfterCompletion c;
> OutOfLineCtor o;
> DerivedFromImported d;
> ImportedMethod m;
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160919/fbff7475/attachment-0001.html>
More information about the cfe-commits
mailing list