LGTM.

rebased.

as DWARFv5 is not fully supported yet - probably not run input and output verifier for 5 version?

rebased.

rebased.

Thank you for the review!

addressed comments.

In D132455#4197342, @JDevlieghere wrote:

In D132455#4106255, @avl wrote:

ping.

@JDevlieghere @aprantl Do you think it is better to move this ConcurrentHashtable into the DWARFLinkerParallel folder?

I'm fine either way. If someone has concerns about the implementation, it's probably less contentious to land it in the DWARFLinkerParallel first, but so far I've not heard any objections. If we do want to use this from LLDB, then we'll need it to be in ADT eventually, though I'd like to do a comparison with Steven's HashMappedTrie for LLDB's real world usage.

Thank you for the review!

Thanks!

Thank you for the review!

already fixed by D80163.

friendly ping.

Thanks for the review!

ping.

Thanks!

addressed comment(added new function doesFormBelongToClass to not depend on DWARFUnit existance)

In D145499#4175487, @dblaikie wrote:

Sounds OK, though might be nice to avoid having these different ways of using (with/without DWARFUnit) and find some way to unify them (have some more general abstraction over DWARFUnit that can be provided by all users of this API or something)

friendly ping.

addressed comments(renamed class methods).

friendly ping.

friendly ping.

@aprantl Thank you for the review!

@aprantl @JDevlieghere @dblaikie @MaskRay Would you mind to take a look at this review, please?

closed in favor of D132455

rebased. added test set - dwarffile.

fixed build.

rebased.

rebased. refactored&simplified.

friendly ping...

ping.

ping.

Thank you for the review!

ping.

@JDevlieghere @aprantl Do you think it is better to move this ConcurrentHashtable into the DWARFLinkerParallel folder?

In D143269#4103095, @friss wrote:

This feels like a nice generalization (and I like the shape of the new code better), but I'm wondering if it's not too general. For example a DT_AT_location can be DW_FORM_addr, and applying PCOffset to it would be wrong. I'm not sure what PCOffset is set to in a variable DIE, but it does feel somewhat wrong even if it's 0. I cannot be 100% sure, but I believe that fear of mishandling non-PC addresses was the original thinking that led to spelling out all the supported cases.

rebased.

rebased.

simplified AddressRangesMap::insert().

https://github.com/llvm/llvm-project/issues/60307

In D142317#4085343, @avl wrote:

In D142317#4083477, @andrewng wrote:

@avl, could you please get this fix cherry picked to the 16.x release branch. The instructions for doing so are here: Backporting Fixes to the Release Branches and the GitHub milestone is here: LLVM 16.0.0 Release. Thanks.

Will do. Thanks.

In D142317#4083477, @andrewng wrote:

@avl, could you please get this fix cherry picked to the 16.x release branch. The instructions for doing so are here: Backporting Fixes to the Release Branches and the GitHub milestone is here: LLVM 16.0.0 Release. Thanks.

ping.

rebased. renamed.

In D140841#4061767, @avl wrote:

In D140841#4059769, @JDevlieghere wrote:

LGTM for the DwarfLinker.

Alexey, how hard would it be to port over the ConstString implementation to use the ConcurrentHashTable?

I think it should not be hard. Thing which I am not sure at the moment is - would it be OK to have thread local allocators for lldb. Anyway, I will prepare a patch and let`s see.

@andrewng @MaskRay Thank you for the review!

In D142317#4076553, @andrewng wrote:

I think you need to be a bit careful with terminology because IIUC it's not actually "thread-safe" but a "per thread" allocator. I think this could be useful in some situations but perhaps a better overall approach would be to use a low-level thread aware memory allocator. I think some toolchains/runtimes already have such an allocator and there are other options such as rpmalloc and mimalloc. This way, the benefit is more widespread and doesn't require any extra effort (although some allocator libraries have some limitations on certain platforms).

addressed comments.

In D142318#4074207, @steven_wu wrote:

I agree that this probably doesn't work for CAS since the use of CAS is not bounded to any context like a ThreadPoolExecutor, for example, it is currently a legal use case to have multiple thread pool to insert into CAS at the same time. It is not feasible to put such a restriction on CAS since CAS should be safe to read/write concurrently from different process.

For this PoolAllocator, it might be a good idea to require it to be initialized with an instance of ThreadPoolExecutor and maybe add a method for Thread to get the context of its Executor so you can double check the allocator is used in the correct Exectuor when assertion is on.

See comment in place.