[Cherry-Pick] Patches for DWARFLinker and DebugNames #8115
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ith ranges When output range is only one entry, and input is low_pc/high_pc do not convert to ranges. This helps with size of .debug_ranges/.debug_rnglists. It also helps when either low_pc/high_pc is 0. We not generating potentially invalid ranges that result in LLDB error. Also fixed handling of DW_AT_subprogram with ranges. This can be created with -fbasic-block-sections=all. Reviewed By: maksfb Differential Revision: https://reviews.llvm.org/D156374 (cherry picked from commit 75f770a)
Clang can generate DW_TAG_inlined_subroutine with low_pc 0. With split dwarf this led to range offset being a negative number. Reviewed By: maksfb Differential Revision: https://reviews.llvm.org/D156742 (cherry picked from commit 9eb0df3)
Now that we have new DWARF Rewriter we can remove DW_AT_low_pc when converting DW_AT_low_pc/DW_AT_high_pc to DW_AT_ranges. Which closer follows DWARF spec. Leaving CU DW_AT_low_pc in place. Reading the spec I think it's needed. Reviewed By: maksfb Differential Revision: https://reviews.llvm.org/D156957 (cherry picked from commit efb8a1c)
Fixed a bug where when Skelton CU had DW_AT_ranges, it the output CU DW_AT_ranges offset was relative, and not absolute. Reviewed By: maksfb Differential Revision: https://reviews.llvm.org/D156958 (cherry picked from commit e1ceae4)
This is purely to make debugging easier for developers. Now that we moved to IR the print out of DIEs is lacking. This function will lazily parse DIE and use DWARFDie dump function. Reviewed By: Amir Differential Revision: https://reviews.llvm.org/D157354 (cherry picked from commit 9ffdc2b)
Changed to creating a new index all the time. This code was legacy of when we couldn't change the size of .debug_info, and led to subtle bugs where index for new entries was pointing to a wrong address. Reviewed By: Amir Differential Revision: https://reviews.llvm.org/D157355 (cherry picked from commit 96cfc5f)
Changed to creating a new index all the time. This code was legacy of when we couldn't change the size of .debug_info, and led to subtle bugs where index for new entries was pointing to a wrong address. Reviewed By: Amir Differential Revision: https://reviews.llvm.org/D157356 (cherry picked from commit 55a1d95)
This bug crept in when CU partitioning was introduced. It manifests itself when there are CUs that use location lists that come before CUs that are part of thin-lto. BOLT processes CUs with cross CU references first (these are produced by thin-lto). When we wrote out all the location lists we did it in original order. Since DWARF4 uses offsets directly in to .debug_loc those offsets in DIEs became wrong. Reviewed By: Amir Differential Revision: https://reviews.llvm.org/D157908 (cherry picked from commit bce5743)
Reviewed By: maksfb Differential Revision: https://reviews.llvm.org/D157206 (cherry picked from commit d7add58)
Create BinaryFunction::translateInputToOutputRange() and use it for updating DWARF debug ranges and location lists while de-duplicating the existing code. Additionally, move DWARF-specific code out of BinaryFunction and add print functions to facilitate debugging. Note that this change is deliberately kept "bug-level" compatible with the existing solution to keep it NFCI and make it easier to track any possible regressions in the future updates to the ranges-handling code. (cherry picked from commit 6e26246)
This is a follow up patch after .debug_names can now emit local type unit entries when we compile with type units + DWARF5 + .debug_names. The pull request that added this functionality was: llvm#70515 This patch makes sure that the DebugNamesDWARFIndex in LLDB will not manually need to parse type units if they have a valid index. It also fixes the index to be able to correctly extract name entries that reference type unit DIEs. Added a test to verify things work as expected.
(cherry picked from commit 2570c7e)
…tion (llvm#74909) This adds support to help LLDB when binary is built with split dwarf, has .debug_names accelerator table and DWP file. Final linked binary might have Type Units (TUs) with the same type signature in multiple compilation units. Although the signature is the same, TUs are not guranted to be bit identical. This is not a problem when they are in .o/.dwo files as LLDB can find them by looking at the right one based on DW_AT_comp_dir/DW_AT_name in skeleton CU. Once DWP is created, TUs are de-duplicated, and we need to know from which CU remaining one came from. This approach allows LLDB to figure it out, with minimal changes to the rest of the tooling. As would have been the case if .debug_tu_index section in DWP was modified.
Add support to the DWARF linkers for emitting DWARF 5 MD5 checksum in the line table. (cherry picked from commit dbea538)
) Specializations of AccelTableBase are always interested in accessing the derived versions of their data classes (e.g. DWARF5AccelTableData). They do so by sprinkling `static_casts` all over the code. This commit adds a helper function to simplify this process, reducinng the number of casts that have to be made in the middle of code, making it easier to read. (cherry picked from commit e72c716)
…kerParallel to have a common library. Part 1. (llvm#75925) This patch creates DWARFLinkerBase library, places DWARFLinker code into DWARFLinker\Classic, places DWARFLinkerParallel into DWARFLinker\Parallel. updates BOLT to use new library. This patch is NFC. (cherry picked from commit 2357e89)
…llvm#77497) This patch backports llvm#77016 into the DWARFLinkerParallel. (cherry picked from commit a6b5d6d)
…lvm#77604) This patch is extracted from llvm#74725. Put some usefull routines into the common Utils.h. (cherry picked from commit a02c0d9)
…lvm#77592) It was noted that new DWARFLinker libraries do not follow naming agreement - llvm#75925 (comment) This patch rename libraries to match with the agreement. Rename LLVMDWARFLinkerBase library into the LLVMDWARFLinker. Rename LLVMDWARFLinker library into the LLVMDWARFLinkerClassic. Correct include path according to the new directory structure. (cherry picked from commit 35708b0)
(cherry picked from commit 44aa4d7)
… vendor names. (llvm#78135) This patch renames values of dsymutil/llvm-dwarfutil options: --linker apple -> --linker classic --linker llvm -> --linker parallel The purpose to rename options is to avoid using vendor names and to match with library names. It should be safe to rename options at current stage as they are not seemed widely used(we may not preserve backward compatibility).
…uesMap. (llvm#77437) This patch is extracted from llvm#74725. Both dwarflinkers contain similar classes for indexed values. Move the code into the DWARFLinkerBase. (cherry picked from commit cf799b3)
…lvm#77932) This patch is extracted from llvm#74725. The DwarfStreamer interface looks overcomplicated and has unnecessary dependencies. This patch avoids creation of DwarfStreamer by DWARFLinker and simplifies interface. (cherry picked from commit 9ff4be6)
This implements the ideas discussed in [1].
To summarize, this commit changes AsmPrinter so that it outputs
DW_IDX_parent information for debug_name entries. It will enable
debuggers to speed up queries for fully qualified types (based on a
DWARFDeclContext) significantly, as debuggers will no longer need to
parse the entire CU in order to inspect the parent chain of a DIE.
Instead, a debugger can simply take the parent DIE offset from the
accelerator table and peek at its name in the debug_info/debug_str
sections.
The implementation uses two types of DW_FORM for the DW_IDX_parent
attribute:
1. DW_FORM_ref4, which points to the accelerator table entry for the
parent.
2. DW_FORM_flag_present, when the entry has a parent that is not in the
table (that is, the parent doesn't have a name, or isn't allowed to be
in the table as per the DWARF spec). This is space-efficient, since it
takes 0 bytes.
The implementation works by:
1. Changing how abbreviations are encoded (so that they encode which
form, if
any, was used to encode IDX_Parent)
2. Creating an MCLabel per accelerator table entry, so that they may be
referred by IDX_parent references.
When all patches related to this are merged, we are able to show that
evaluating an expression such as:
```
lldb --batch -o 'b CodeGenFunction::GenerateCode' -o run -o 'expr Fn' -- \
clang++ -c -g test.cpp -o /dev/null
```
is far faster: from ~5000 ms to ~1500ms.
Building llvm-project + clang with and without this patch, and looking
at its impact on object file size:
```
ls -la $(find build_stage2_Debug_idx_parent_assert_dwarf5 -name \*.cpp.o) | awk '{s+=$5} END {printf "%\047d\n", s}'
11,507,327,592
-la $(find build_stage2_Debug_no_idx_parent_assert_dwarf5 -name \*.cpp.o) | awk '{s+=$5} END {printf "%\047d\n", s}'
11,436,446,616
```
That is, an increase of 0.62% in total object file size.
Looking only at debug_names:
```
$stage1_build/bin/llvm-objdump --section-headers $(find build_stage2_Debug_idx_parent_assert_dwarf5 -name \*.cpp.o) | grep __debug_names | awk '{s+="0x"$3} END {printf "%\047d\n", s}'
440,772,348
$stage1_build/bin/llvm-objdump --section-headers $(find build_stage2_Debug_no_idx_parent_assert_dwarf5 -name \*.cpp.o) | grep __debug_names | awk '{s+="0x"$3} END {printf "%\047d\n", s}'
369,867,920
```
That is an increase of 19%.
DWARF Linkers need to be changed in order to support this. This commit
already brings support to "base" linker, but it does not attempt to
modify the parallel linker. Accelerator entries refer to the
corresponding DIE offset, and this patch also requires the parent DIE
offset -- it's not clear how the parallel linker can access this. It may
be obvious to someone familiar with it, but it would be nice to get help
from its authors.
[1]:
https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/
(cherry picked from commit b667783)
This patch adds check for parallel linker to tests located in the root of test/tools/dsymutil. (cherry picked from commit 0ed8194)
This commit introduces a helper function to DWARFAcceleratorTable::Entry which follows DW_IDX_Parent attributes to returns the corresponding parent Entry in the table. It is tested by enhancing dwarfdump so that it now prints: 1. When data is corrupt. 2. When parent information is present, but the parent is not indexed. 3. The parent entry offset, when the parent is present and indexed. This is printed in terms a real entry offset (the same that gets printed at the start of each entry: "Entry @ 0x..."), instead of the encoded number in the table (which is an offset from the start off the Entry list). This makes it easy to visually inspect the dwarfdump and check what the parent is. (cherry picked from commit 380ac53)
The current implementation of DebugNames is _only_ using hashes to compute the bucket number. Once inside the bucket, it reverts back to string comparisons, even though not all hashes inside a bucket are identical. This commit changes the behavior so that we check the hash before comparing strings. Such check is so important that it speeds up a simple benchmark by 20%. In other words, the following expression evaluation time goes from 1100ms to 850ms. ``` bin/lldb \ --batch \ -o "b CodeGenFunction::GenerateCode" \ -o run \ -o "expr Fn" \ -- \ clang++ -c -g test.cpp -o /dev/null &> output ``` (Note, these numbers are considering the usage of IDX_parent) (cherry picked from commit 69cb99f)
TableEntry names are pointers into the string table section, and accessing their length requires a search for `\0`. However, 99% of the time we only need to compare the name against some other other, and such a comparison will fail as early as the first character. This commit adds a method to the interface of TableEntry so that such a comparison can be done without extracting the full name. It saves 10% in the time (1250ms -> 1100 ms) to evaluate the following expression. ``` lldb \ --batch \ -o "b CodeGenFunction::GenerateCode" \ -o run \ -o "expr Fn" \ -- \ clang++ -c -g test.cpp -o /dev/null &> output ``` (cherry picked from commit 75ea78a)
This function has no mutable behavior (cherry picked from commit 8f40783)
This allows us to query the AT_Name of a DIE without parsing the entire CU. Part of the ongoing effort to support IDX_Parent in accelerator tables [1]. [1]: https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/44 (cherry picked from commit 4684507)
|
@swift-ci test |
adrian-prantl
approved these changes
Feb 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is another series of patches containing a number of fixes done to the dwarf linker, including required re-structuring of the files that was done upstream. The Bolt commits are pulled together to avoid merge conflicts when cherry-picking.
This should be the last of batch, there are two patches are still waiting for upstream reviews.