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

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

In D96035#2579263 <https://reviews.llvm.org/D96035#2579263>, @avl wrote:

> @JDevlieghere @aprantl @echristo @dblaikie @clayborg
>
> What do you think of this patch? Is it OK in general and Is it worth to divide it into smaller patches and integrate?

The original dsymutil (pre llvm version) that I wrote was deterministic, and so is the current one. That being said, I am not too worried about this because the quality of the resulting debug info is the main goal here.

My main concerns are:

- 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?
- 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?
- 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.

I do your idea of creating a new compile unit for types in order to make ODR better as I have thought of this. A few extra ideas on this:

- it might be best to just create type units on the fly instead of making standard DWARF?
- if we don't want to create/use type units then:
  - create a new DW_TAG_compile_unit whose source path is the DW_AT_decl_file for the type in question
  - Bonus points if we can add all types form the same DW_AT_decl_file to the same DW_TAG_compile_unit
  - 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. Right now when ODR finds type "class A" with no compiler created constructor, it will re-emit a full type definition because DWARF needs to be able to reference it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96035