[PATCH] D76085: [DWARFLinker][dsymutil][NFC] add section index into address range.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D76085#1921389 <https://reviews.llvm.org/D76085#1921389>, @avl wrote:

> > My only rough thought here (as I'm not especially versed in the DWARFLinker code) is that this feature (doing debug info redundancy elimination during ld -r) seems more like a stretch/later goal & I'd have thought it'd be likely/better to implement this for non"-r" linking first, since that's close(r) to the llvm-dsymutil functionality already in DWARFLinker, then in subsequent patches generalize this (in the way you're proposing) to handle the "-r" sort of case, which is probably (likely/actually) more complicated (or at least more new code, since it's not something that DWARFLuinker has had to support until this point), keeping track of addresses, etc.
>
> At this point, I do not plan to support ld -r, since "-r and --gc-sections may not be used together". So this change is for non "-r" linking.
>
> I think it is better not to implement "debug info redundancy elimination in lld" using the standard dsymutil scenario. Instead, it is better to use an optimized scenario:
>
> 1. standard dsymutil scenario:
> 2. lld creates liveness map for sections.
> 3. lld resolves relocations and generates contents of output sections.
> 4. lld creates a debug map for DWARFLinker.
> 5. DWARFLinker links debug info(creates a tree of cloned DIEs).
> 6. DWARFLinker generates new content for output sections.
>
>   Memory objects:
> 7. source debug info sections.
> 8. NON-optimized debug info sections.


I'm not sure where this ^ happens in the scenario list you describe above. And is this fundamentally necessary for the llvm-dsymutil functionality, but not needed for lld? Or is this an improvement that could be made (& tested) in llvm-dsymutil independently/prior to the lld work?

> - DIEs tree.
> - optimized output debug info sections.
> - optimized dsymutil scenario:
> - lld creates liveness map for sections.
> - DWARFLinker links debug info(creates a tree of cloned DIEs).
> - DWARFLinker generates new content for output sections.
> - lld resolves relocations and generates contents of output sections.
> 
>   Memory objects:
> - source debug info sections.
> - DIEs tree.
> - optimized debug info sections.
> - optimized output debug info sections.
> 
>   The second scenario does not need to create a debug map. It also requires less memory. Thus I am planning to use this second scenario.
> 
>   If we would do the first scenario and later would implement second, then all lld integration part should be thrown away and replaced with the new implementation. So implementing the first scenario would be just a waste of time.
> 
>   I think it makes sense to implement the second scenario from scratch.
> 
>> Could changes like this be tested (possibly unit tested) in some way? I guess DWARFLinker funcitonality's been tested in llvm-dsymutil up until this point - but this is the first major/new functionality DWARFLinker will support that llvm-dsymutil does not support, so some in-tree testing story will need to be created.
> 
> Yes. I am going to add tests together with new functionality.

Changes to code in the llvm/ repository should generally be tested that repository - not only by tests in another project like lld.

> My overall development plan looks like this :
> 
> 1. create a first version that would work for .debug_info/.debug_ranges tables. This version could be taken for evaluation, as requested in D74169 <https://reviews.llvm.org/D74169>.
>   1. D76085 <https://reviews.llvm.org/D76085> patch.
>   2. internal patch implementing AddressesMap in lld.
>   3. internal patch implementing DebugInputSection in lld.
>   4. internal patch dumping DIEs into bytes array.
>   5. internal patch implementing DwarfEmitter(dump DIEs into bytes array).
> 2. divide the above patch into smaller patches and integrate them with a set of tests.
> 3. implement the rest of the debug info tables.
> 
>   So I assumed that D76085 <https://reviews.llvm.org/D76085> would be done as NFC for the current codebase. And changes in interfaces would be used while integrating patches on step 2.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76085