[PATCH] D158124: [dsymutil] Add support for mergeable libraries

Alpha Abdoulaye via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 30 15:25:31 PDT 2023


Alpha added a comment.

I have nothing necessarily against the suggestion to help the parallel implementation, only a few points:

In D158124#4622768 <https://reviews.llvm.org/D158124#4622768>, @avl wrote:

> 2. When the AddressesMap object is created - build a map for units relocations:
>
>   DenseMap<uint64_t Offset, std::vector<ValidReloc>> StoredValidDebugInfoRelocsMap;
>   
>   DwarfLinkerForBinary::AddressManager<AddressesMapBase> (DWARFContext& Context) {
>   
>     for (const std::unique_ptr<DWARFUnit> &CU : Context.compile_units()) {
>       StoredValidDebugInfoRelocs.insert(std::make_pair(CU->getOffset(), std::vector<ValidReloc>>));
>     }

In dsymutil, the `DWARFContext` is not passed to the `AddressManager`.
I could break down the DwarfFile constructor within `DwarfLinkerForBinary::loadObject`, and pass the second DwrafContext argument also to the `AddressesMap` as a new parameter, if we feel that is reasonable architecturally and would not lead to unknown race and/or runtime dependency issues (?)

> 4. Add the cleanupInputData() method to the AddressesMap which will release all resources except StoredValidDebugInfoRelocsMap. that allows calling "Addresses.cleanupInputData()" instead of "Addresses.reset();" and then to keep StoredValidDebugInfoRelocsMap for further processing.

Isn't this already the purpose of `clear()` ? Like the following from `DwarfLinkerForBinary.h`

  void clear() override {
    ValidDebugInfoRelocs.clear();
    ValidDebugAddrRelocs.clear();
  }




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

https://reviews.llvm.org/D158124



More information about the llvm-commits mailing list