r184473 - Debug Info: Attempt to resolve forward declarations if we are not emitting

[David Blaikie]

On Thu, Jun 20, 2013 at 2:17 PM, Adrian Prantl <aprantl at apple.com> wrote:
> Author: adrian
> Date: Thu Jun 20 16:17:24 2013
> New Revision: 184473
>
> URL: http://llvm.org/viewvc/llvm-project?rev=184473&view=rev
> Log:
> Debug Info: Attempt to resolve forward declarations if we are not emitting
> limited debug info.

As discussed in Keith's thread ("[PATCH] Output debug information for
structure members only referenced by a pointer or typedef") I have
been looking into this (I suppose I could've assigned PR16214 to me)

Wouldn't've minded discussing this approach first, but either way will
do I guess.

> This is another small addendum to r184252.
>
> rdar://problem/14101097

PR16214.

>
> Added:
>     cfe/trunk/test/CodeGen/debug-info-record2.c
>       - copied, changed from r184442, cfe/trunk/test/CodeGen/debug-info-record.c
> Modified:
>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>     cfe/trunk/test/CodeGen/debug-info-record.c
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=184473&r1=184472&r2=184473&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Jun 20 16:17:24 2013
> @@ -1886,6 +1886,22 @@ llvm::DIType CGDebugInfo::getTypeOrNull(
>    return llvm::DIType();
>  }
>
> +/// ContainsFwdDecl - True if Ty is either a forward declaration or a
> +/// derived type of a forward declaration.
> +static bool ContainsFwdDecl(llvm::DIType Ty) {
> +  // Composite types such as structs inherit from DIDerivedType, so
> +  // check this first and do an early exit.
> +  if (Ty.isForwardDecl())
> +    return true;
> +
> +  if (Ty.isDerivedType()) {
> +    llvm::DIDerivedType DTy(Ty);
> +    return ContainsFwdDecl(DTy.getTypeDerivedFrom());

Is recursion really the right tool here? Seems a bit gratuitous.

> +  }
> +
> +  return false;
> +}
> +
>  /// getCompletedTypeOrNull - Get the type from the cache or return null if it
>  /// doesn't exist.
>  llvm::DIType CGDebugInfo::getCompletedTypeOrNull(QualType Ty) {
> @@ -1904,8 +1920,17 @@ llvm::DIType CGDebugInfo::getCompletedTy
>    }
>
>    // Verify that any cached debug info still exists.
> -  if (V != 0)
> -    return llvm::DIType(cast<llvm::MDNode>(V));
> +  if (V != 0) {
> +    llvm::DIType CachedType(cast<llvm::MDNode>(V));
> +
> +    // Unless we are limiting debug info, attempt to resolve any
> +    // forward declarations.
> +    if (DebugKind > CodeGenOptions::LimitedDebugInfo &&
> +        ContainsFwdDecl(CachedType))
> +      return llvm::DIType();

So this doesn't account for declaration V definition - this means if
we repeatedly need the declaration we'll keep building new
declarations of the original type and its derived types. This "works
out" because of metadata uniquing, but leaves me a bit uncomfortable.

This also doesn't apply to pointers, either - we'll needlessly rebuild
pointer types every time even though we could never want to emit the
definition of the thing they point to, we only ever need the
declaration.

I'd sort of rather we take a more explicit approach, but I've not
figured out exactly what will work nicely yet.

> +
> +    return CachedType;
> +  }
>
>    return llvm::DIType();
>  }
>
> Modified: cfe/trunk/test/CodeGen/debug-info-record.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-record.c?rev=184473&r1=184472&r2=184473&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/debug-info-record.c (original)
> +++ cfe/trunk/test/CodeGen/debug-info-record.c Thu Jun 20 16:17:24 2013
> @@ -1,5 +1,5 @@
>  // RUN: %clang_cc1 -triple x86_64-unk-unk -fno-limit-debug-info -o - -emit-llvm -g %s | FileCheck %s
> -// Check that we emit debug info for a struct even if it is typedef'd before using.
> +// Check that we emit debug info for a struct even if it is typedef'd.
>  // rdar://problem/14101097
>  //
>  // FIXME: This should work with -flimit-debug-info, too.
>
> Copied: cfe/trunk/test/CodeGen/debug-info-record2.c (from r184442, cfe/trunk/test/CodeGen/debug-info-record.c)
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-record2.c?p2=cfe/trunk/test/CodeGen/debug-info-record2.c&p1=cfe/trunk/test/CodeGen/debug-info-record.c&r1=184442&r2=184473&rev=184473&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/debug-info-record.c (original)
> +++ cfe/trunk/test/CodeGen/debug-info-record2.c Thu Jun 20 16:17:24 2013
> @@ -8,11 +8,12 @@
>  // CHECK-NOT: [ DW_TAG_structure_type ] [elusive_s] {{.*}} [fwd]
>  // CHECK: [ DW_TAG_member ] [foo]
>  // CHECK: [ DW_TAG_member ] [bar]
> +typedef struct elusive_s* elusive_t;
> +elusive_t first_use = (void*)0;
>  struct elusive_s {
>    int foo;
>    float bar;
>  };
> -typedef struct elusive_s* elusive_t;
>
>  int baz(void* x) {
>    elusive_t s = x;
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits