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

[Alexey Lapshin via llvm-commits]

On 05.02.2021 21:55, David Blaikie wrote:
> On Fri, Feb 5, 2021 at 7:38 AM Alexey Lapshin via Phabricator
> <reviews at reviews.llvm.org> wrote:
>
>> 1. In multi-thread mode, some canonical dies might become not seen by some threads.
>>
>>    If the request for setting canonical dies come from several threads at the same time - some threads might receive information that there is not canonical die(though it is actually, it would be set by another thread). As the result, some type would not be de-duplicated. But the number of such cases is small. For the clang, there is no noticeable difference in the size of the binary.
> That sounds concerning - does this mean the result would be
> nondeterministic? Determinism is an important feature.

yep. the result would be nondeterministic for the case --num-threads != 1.

For the case --num-threads == 1, the result would be deterministic.

The original idea : mark the first met type description as canonical and 
set all following references to that canonical type - could not be 
deterministic for the multi-thread case. Because there is no fixed order 
of execution for threads and first met type description might differs.

Not for that patch, but just as a note. Implementing type merging and 
using global type table will made a process to be deterministic even for 
-num-threads != 1. We might consider it as a future improvement.


>
>>>> e) .debug_names is not supported by this patch.
>>> Any reason why this can't be done? I believe that dsymutil just generates the .debug_names (or Apple accelerator tables) after you have written all of the DWARF out by traversing it no? If this doesn't happen, it can be added with the --update option to dsymutil, but again, this takes time.
>> There is no fundamental reason. I think it might be supported.
>> I just need more time to check that code and understand how to implement it properly.
>> I would update this patch with implementation later.
> Yeah, if it's not already supported (don't think it is), it'd be good
> to do it separately/follow-up patch. Smallest patches making
> incremental progress (without having to go through too many
> convolutions/rewrites/etc) make it easier to review, etc.

.debug_names is supported by current dsymutil. But it is not supported 
by this patch.

and yes - it would be good to add this functionality in separate patch.


Thank you, Alexey.