[PATCH] D96035: [WIP][dsymutil][DWARFlinker] implement separate multi-thread processing for compile units.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

> The final .debug_str should only have one copy of a string. It should be possible to create individual string tables and merge them in the last loop you have that fixes up offsets right?

Right. The final .debug_str has only one copy of a string.
The implementation is a bit different though:

While cloning compile unit, DWARFLinker remembers the pointer to the string and reference to the string attribute:

  // create delayed task for setting proper offset value for the string
  // attribute.
  CU.noteOffsetData(It, *String);

After cloning is complete, DWARFLinker puts remembered strings into the common StringPool and attribute is updated:

  void DWARFLinker::assignDelayedOffsets(CompileUnit &CU) {
    std::unique_lock<std::mutex> Guard(StringPoolMutex);
  
    for (const CompileUnit::OffsetsData &Data : CU.getStringsOffsetData())
      setStringsOffset(Data.AttrToPatch, Data.String);
  }

This way common resource TheOffsetsStringPool is locked only once per compile unit.
After all compile units are processed the .debug_str is generated from the TheOffsetsStringPool
which has only one copy of each string.

> I am unclear on how ODR uniquing works here without cross talking between threads and slowing everything down. Since they currently create new DW_FORM_ref_addr that reference other already streamed out CUs, how does that work in this patch?

ODR uniquing cross talks between threads. It is implemented this way:

During analyzing declaration context stage DWARFLinker remembers properties of each declaration context for compile unit:

  // In multy-threaded case it is in-effective to lock&use
  // common resource ODRContexts.getChildDeclContext() for
  // every type declaration context. Instead, we remember
  // DeclarationContextKey and create DeclarationContext and
  // update Info.Ctxt field later. That way we lock
  // ODRContexts.getChildDeclContext() only once for each compile
  // unit.
  
  CU.noteContextData(Current.ContextKey, &Info);

Later, after all types are analyzed, corresponding type declaration contexts are created in the common 
ODRContexts pool.

  void DWARFLinker::assignDelayedDeclContexts(CompileUnit &CU) {
    std::unique_lock<std::mutex> Guard(ContextsMutex);
  
    for (const CompileUnit::DeclContextKeyData &Data : CU.getDeclContextData())
      setDeclContext(Data.Key, Data.Info);
  }

At that stage, Declaration contexts(which are already is in ODRContexts pool) might be reused.

> I am unclear on what state the inter CU references are in in this patch when they are in the same .o file from LTO. Do they work? Are you loading all CUs into memory? I would be ok with loading all CUs from the same file at one time.

inter CU references are supported. This patch tries to load/unload not dependent compile units and load at one time only inter-connected compile units. 
Let`s we have following dependencies:

CU1 references CU2, CU2 references CU4, CU4 references CU3, CU5(does not reference other CUs)

1. first pass. CU1 loaded. It is seen that it needs CU2, so left CU1 in memory. CU2 loaded. It is seen that it needs CU4, so left CU2 in memory. CU3 loaded. processed. unloaded. CU4 loaded. It is seen that it needs CU3, so left CU4 in memory. CU5 loaded. processed. unloaded.
2. second pass. CU1 processed. CU2 processed. CU3 loaded. processed. CU4 processed.

The reason why I try to avoid loading all CUs from the same file at one time, this utility - https://reviews.llvm.org/D86539.
Implementation which loads all CUs from the same file at one time would require a lot of memory if it optimize single binary(not a set of object files).

> it might be best to just create type units on the fly instead of making fake compile units?

Type units might be created, but they require additional space. We might lose some size improvement.

> create a new DW_TAG_compile_unit whose source path is the DW_AT_decl_file for the type in question

I thought about single artificial compile unit for all types, but yes we might implement it this way.

> Some type class declarations that use compiler default created constructors, copy constructor, assignment operators might omit these member functions in various different compile units if they were or were not used by that compile unit. If we can augment the uniqued type definition to include these, that would be a huge win.

yes. That is the idea for type merging. it might save a lot of space.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96035/new/

https://reviews.llvm.org/D96035