[PATCH] D19236: Add DITypeIndex for CodeView and emit it for locals

[David Blaikie via llvm-commits]

On Tue, Apr 19, 2016 at 2:20 PM, Reid Kleckner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> rnk marked 2 inline comments as done.
> rnk added a comment.
>
> In http://reviews.llvm.org/D19236#405489, @aaboud wrote:
>
> > I am a little confused.
> >
> > 1. Are you trying to replace DIBasicType?
>
>
> Yes
>
> > 2. Will Clang be generating DITypeIndex for Windows target (COFF debug
> info) and DIBasicType/DICompositeType/DIDerivedType/DISubroutineType for
> Linux target (DWARF debug info)?
>
>
> Yes
>
> > 3. You did not explain how you will represent the compound types using
> this DITypeIndex!
>
>
> My plan is that we will have named metadata, like llvm.dbg.cvtypes, which
> will be a list of pairs of CUs and type streams. The type stream will be a
> simple MDString. This avoids having DICompileUnit reference the type
> streams directly, while still allowing us to associate the type stream with
> the CU. This is similar to how we associated subprograms with CUs currently.
>

What's the goal/advantage of having CUs not directly reference type streams?


>
> Later we can add a specialized DI node to represent type streams
> (DITypeStream/Blob?) if we think it's worth it. It's not very efficient for
> the entire type stream to participate in MDString uniquing, for example,
> and printing a bunch of binary hex escapes is very ugly.
>
> >   3.a. Who will set the index field in DITypeIndex for compound types?
> can clang FE do that?
>
>
> Yes, the frontend will drive llvm::codeview::MemoryTypeTableBuilder to do
> this.
>
> >   3.b. Will CodeView emitter emit that index directly? what will happen
> if we do not want to emit some of the types, do not we need to rearrange
> the indices?
>
>
> Yes, if you want to change or drop some type information, your tool will
> need to be format-aware. LTO doesn't need to do this, it can simply
> concatenate the llvm.dbg.cvtypes lists. The backend can do the merging and
> renumbering when it emits .debug$T.
>

Practically speaking, LTO will likely need to be format-aware for
practical/realistic performance reasons (the same reason all the work has
gone into deduplicating types in the current metadata scheme - without it,
LTO is impractical).


>
> >   3.c. Will this work also for emitting a pdb file? can the compiler set
> these indices also when emitting types to pdb file instead of COFF object
> file?
>
>
> I have no plans to implement MSVC /Zi-style PDB emission, where the
> compiler communicates with a PDB server process that brokers type indices.
> If you did this, I could easily imagine replacing the type stream with a
> sentinel that says "this type index refers to that PDB".
>
> I *do* want clang to avoid duplicating type information when the type was
> defined in another module, similar to what we do for DWARF. For now if we
> detect that case, we can emit a forward declaration for the type in the
> type stream to give it a type index. In the future, we may want a way to
> represent a type index from some stream other than that of the current CU.
>
>
> ================
> Comment at: include/llvm/IR/DebugInfoMetadata.h:2391
> @@ +2390,3 @@
> +/// Represents a type in a DITypeStream at the given index. Uses the
> +/// DITypeStream in the current CU. Doesn't contain metadata operands, so
> this
> +/// inherits directly from Metadata.
> ----------------
> The DICompileUnit. I want LTO to be functionally correct, if inefficient,
> in the presence of this data. For now, this means having two CUs and two
> type streams in one module, and fixing up the type indices in the backend.
> We can teach LTO to merge the type streams later.
>
>
> http://reviews.llvm.org/D19236
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160419/f68c88fe/attachment.html>