Page MenuHomePhabricator

[LLD][COFF] DebugTypes prototype -- NOT FOR SUBMIT
Changes PlannedPublic

Authored by aganea on Mar 11 2019, 11:59 AM.

Details

Reviewers
rnk
zturner
Summary

(this patch is only for RFC purposes)

Rationale

The long term objective here is to parallelize the PDB Type merging.

OBJ files can sometimes have a dependence on other files (precomp OBJ or PDB type server).
Once discovered, those dependencies (precomp OBJ and PDB type server) need to be merged before the dependent OBJ are merged.

Previously, this was done on the fly - whenever a dependence was encountered it was loaded in.
Of course in a single threded environment this works, but not once we want to introduce multithreading, like demonstrated in D55585.

PDB type server as-an-input

A PDB type server is simply a container for a pre-merged .debug$T stream, produced when compiling with MSVC /Zi. OBJs compiled in such way will reference a PDB type server instead of storing a regular .debug$T stream of their own.

Among other things, this demo patch moves loading of PDB type servers in InputFiles.cpp and treats them as other cmd-line inputs.
A secondary objective is to allow for opening the inputs in a multi-threaded manner in the future (ie. somehow parallelize lld::coff::LinkerDriver::run()).

Debug types

The other major change this patch introduces is a class hierarchy designed to handle the different .debug$T containers, as suggested by @rnk.

class TpiSource; // publicly visible
class TypeServerSource : public TpiSource; // internal, handles PDB type server merging
class UseTypeServerSource : public TpiSource; // internal, handles any OBJ that depends on a PDB type server
class PrecompSource : public TpiSource; // internal, handles a precompiled headers OBJ
class UsePrecompSource : public TpiSource; // inernal, handles any OBJ that depends on a precomp OBJ

This essentially moves all the type merging from PDB.cpp to a new DebugTypes.cpp

Proposed commit strategy

I propose converging this in several steps, each point is an upcoming patch:

  1. Move dependency detection early into InputFiles.cpp out from PDB.cpp; introduce naked DebugTypes hierarchy - D59053
  2. Move PDB type server loading from PDB.cp early into InputFiles.cpp and introduce PDBInputFile - D60095
  3. Introduce TypeMerger.h - D60070
  4. Take out module DBI creation out of PDBLinker::addObjFile() and treat it separately from type merging - D59261
  5. Implement TpiSource::mergeDebugT and move code from PDB.cpp.
  6. Implement TypeServerSource::mergeDebugT and UseTypeServerSource and move code from PDB.cpp
  7. Implement PrecompSource::mergeDebugT and UsePrecompSource and move code from PDB.cpp

Diff Detail

Event Timeline

aganea created this revision.Mar 11 2019, 11:59 AM
aganea created this object with visibility "No One".
aganea changed the visibility from "No One" to "Public (No Login Required)".
aganea added a reviewer: rnk.Mar 11 2019, 12:03 PM
aganea set the repository for this revision to rLLD LLVM Linker.
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2019, 12:04 PM
aganea edited the summary of this revision. (Show Details)Mar 11 2019, 12:57 PM
aganea added a reviewer: zturner.
aganea edited the summary of this revision. (Show Details)Mar 11 2019, 1:44 PM
aganea edited the summary of this revision. (Show Details)
rnk added a comment.Mar 11 2019, 3:58 PM

Yep, as a direction, this makes sense to me.

COFF/DebugTypes.h
40

"DebugT" seems specific to /Z7 object files, I'd just say "mergeTypes". I suppose this technically also merges LF_*_ID records, but I think the meaning is clear. I guess we could say mergeTpi, which includes both kinds.

47

I would recommend llvm::function_ref to avoid extra allocations.

COFF/MergedState.h
1

TypeMerger.h, or whatever class name we come up with? TypeMerging.h, TpiMerging.h?

19

Maybe to be more OO, this should be the TypeMerger, and then we'd add methods to it that do the job? It makes sense that the PDBLinker owns a TypeMerger which... merges type records. Maybe TpiMerger?

aganea planned changes to this revision.Mar 14 2019, 11:48 AM

This won't have any effect for obj files produced by clang-cl, correct? (Since they don't reference external data.)

What's the runtime impact of this patch?

aganea added a comment.EditedApr 1 2019, 8:11 AM

This won't have any effect for obj files produced by clang-cl, correct? (Since they don't reference external data.)

Indeed, this is only for MSVC-generated OBJ files, since clang-cl doesn't support /Zi, and /Yu is handled in a different way as MSVC.

What's the runtime impact of this patch?

As it stands, none. It's just a step towards D55585 and further on, parallelization of type merging.