[Lldb-commits] [clang] [lldb] [llvm] [BOLT][DWARF][NFC] Split processUnitDIE into two lambdas (PR #99957)
Sayhaan Siddiqui via lldb-commits
lldb-commits at lists.llvm.org
Mon Jul 22 18:02:35 PDT 2024
https://github.com/sayhaan updated https://github.com/llvm/llvm-project/pull/99957
>From 11fff0b2d1868f1e8fce988b2b41a38de42dff61 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at meta.com>
Date: Tue, 1 Jun 2021 11:37:41 -0700
Subject: [PATCH 1/4] Rebase: [Facebook] Add clang driver options to test debug
info and BOLT
Summary:
This is an essential piece of infrastructure for us to be
continuously testing debug info with BOLT. We can't only make changes
to a test repo because we need to change debuginfo tests to call BOLT,
hence, this diff needs to sit in our opensource repo. But when upstreaming
to LLVM, this should be kept BOLT-only outside of LLVM. When upstreaming,
we need to git diff and check all folders that are being modified by our
commits and discard this one (and leave as an internal diff).
To test BOLT in debuginfo tests, configure it with -DLLVM_TEST_BOLT=ON.
Then run check-lldb and check-debuginfo.
Manual rebase conflict history:
https://phabricator.intern.facebook.com/D29205224
https://phabricator.intern.facebook.com/D29564078
https://phabricator.intern.facebook.com/D33289118
https://phabricator.intern.facebook.com/D34957174
https://phabricator.intern.facebook.com/D35317341
Test Plan:
tested locally
Configured with:
-DLLVM_ENABLE_PROJECTS="clang;lld;lldb;compiler-rt;bolt;debuginfo-tests"
-DLLVM_TEST_BOLT=ON
Ran test suite with:
ninja check-debuginfo
ninja check-lldb
Reviewers: maks, #llvm-bolt
Reviewed By: maks
Subscribers: ayermolo, phabricatorlinter
Differential Revision: https://phabricator.intern.facebook.com/D46256657
Tasks: T92898286
---
clang/include/clang/Driver/Options.td | 4 ++++
clang/lib/Driver/ToolChains/Gnu.cpp | 29 ++++++++++++++++++++++++++
cross-project-tests/lit.cfg.py | 14 ++++++++++++-
cross-project-tests/lit.site.cfg.py.in | 4 ++++
lldb/test/API/lit.cfg.py | 5 +++++
lldb/test/API/lit.site.cfg.py.in | 8 +++++++
lldb/test/Shell/helper/toolchain.py | 5 +++++
lldb/test/Shell/lit.site.cfg.py.in | 9 ++++++++
llvm/CMakeLists.txt | 4 ++++
9 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9c6cebd77ff0a..8ccbc4b372ba3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5553,6 +5553,10 @@ def pg : Flag<["-"], "pg">, HelpText<"Enable mcount instrumentation">,
MarshallingInfoFlag<CodeGenOpts<"InstrumentForProfiling">>;
def pipe : Flag<["-", "--"], "pipe">,
HelpText<"Use pipes between commands, when possible">;
+// Facebook T92898286
+def post_link_optimize : Flag<["--"], "post-link-optimize">,
+ HelpText<"Apply post-link optimizations using BOLT">;
+// End Facebook T92898286
def prebind__all__twolevel__modules : Flag<["-"], "prebind_all_twolevel_modules">;
def prebind : Flag<["-"], "prebind">;
def preload : Flag<["-"], "preload">;
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 52c2ee90b1b28..ff20deb9c4f86 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -672,12 +672,41 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}
}
+ // Facebook T92898286
+ if (Args.hasArg(options::OPT_post_link_optimize))
+ CmdArgs.push_back("-q");
+ // End Facebook T92898286
+
Args.AddAllArgs(CmdArgs, options::OPT_T);
const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
C.addCommand(std::make_unique<Command>(JA, *this,
ResponseFileSupport::AtFileCurCP(),
Exec, CmdArgs, Inputs, Output));
+ // Facebook T92898286
+ if (!Args.hasArg(options::OPT_post_link_optimize) || !Output.isFilename())
+ return;
+
+ const char *MvExec = Args.MakeArgString(ToolChain.GetProgramPath("mv"));
+ ArgStringList MoveCmdArgs;
+ MoveCmdArgs.push_back(Output.getFilename());
+ const char *PreBoltBin =
+ Args.MakeArgString(Twine(Output.getFilename()) + ".pre-bolt");
+ MoveCmdArgs.push_back(PreBoltBin);
+ C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
+ MvExec, MoveCmdArgs, std::nullopt));
+
+ ArgStringList BoltCmdArgs;
+ const char *BoltExec =
+ Args.MakeArgString(ToolChain.GetProgramPath("llvm-bolt"));
+ BoltCmdArgs.push_back(PreBoltBin);
+ BoltCmdArgs.push_back("-reorder-blocks=reverse");
+ BoltCmdArgs.push_back("-update-debug-sections");
+ BoltCmdArgs.push_back("-o");
+ BoltCmdArgs.push_back(Output.getFilename());
+ C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
+ BoltExec, BoltCmdArgs, std::nullopt));
+ // End Facebook T92898286
}
void tools::gnutools::Assembler::ConstructJob(Compilation &C,
diff --git a/cross-project-tests/lit.cfg.py b/cross-project-tests/lit.cfg.py
index 774c4eaf4d976..619634578dfe6 100644
--- a/cross-project-tests/lit.cfg.py
+++ b/cross-project-tests/lit.cfg.py
@@ -84,7 +84,13 @@ def get_required_attr(config, attr_name):
# use_clang() and use_lld() respectively, so set them to "", if needed.
if not hasattr(config, "clang_src_dir"):
config.clang_src_dir = ""
-llvm_config.use_clang(required=("clang" in config.llvm_enabled_projects))
+# Facebook T92898286
+should_test_bolt = get_required_attr(config, "llvm_test_bolt")
+if should_test_bolt:
+ llvm_config.use_clang(required=("clang" in config.llvm_enabled_projects), additional_flags=["--post-link-optimize"])
+else:
+ llvm_config.use_clang(required=("clang" in config.llvm_enabled_projects))
+# End Facebook T92898286
if not hasattr(config, "lld_src_dir"):
config.lld_src_dir = ""
@@ -293,3 +299,9 @@ def get_clang_default_dwarf_version_string(triple):
# Allow 'REQUIRES: XXX-registered-target' in tests.
for arch in config.targets_to_build:
config.available_features.add(arch.lower() + "-registered-target")
+
+# Facebook T92898286
+# Ensure the user's PYTHONPATH is included.
+if "PYTHONPATH" in os.environ:
+ config.environment["PYTHONPATH"] = os.environ["PYTHONPATH"]
+# End Facebook T92898286
diff --git a/cross-project-tests/lit.site.cfg.py.in b/cross-project-tests/lit.site.cfg.py.in
index 39458dfc79afd..2d53cd377f033 100644
--- a/cross-project-tests/lit.site.cfg.py.in
+++ b/cross-project-tests/lit.site.cfg.py.in
@@ -21,6 +21,10 @@ config.mlir_src_root = "@MLIR_SOURCE_DIR@"
config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
+# Facebook T92898286
+config.llvm_test_bolt = lit.util.pythonize_bool("@LLVM_TEST_BOLT@")
+# End Facebook T92898286
+
import lit.llvm
lit.llvm.initialize(lit_config, config)
diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 96520c7c82624..dfeb76544e57d 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -265,6 +265,11 @@ def delete_module_cache(path):
if is_configured("lldb_framework_dir"):
dotest_cmd += ["--framework", config.lldb_framework_dir]
+# Facebook T92898286
+if is_configured("llvm_test_bolt"):
+ dotest_cmd += ["-E", '"--post-link-optimize"']
+# End Facebook T92898286
+
if (
"lldb-repro-capture" in config.available_features
or "lldb-repro-replay" in config.available_features
diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in
index 8b2d09ae41cd2..602f45759e48f 100644
--- a/lldb/test/API/lit.site.cfg.py.in
+++ b/lldb/test/API/lit.site.cfg.py.in
@@ -1,5 +1,9 @@
@LIT_SITE_CFG_IN_HEADER@
+#Facebook T92898286
+import lit.util
+#End Facebook T92898286
+
config.llvm_src_root = "@LLVM_SOURCE_DIR@"
config.llvm_obj_root = "@LLVM_BINARY_DIR@"
config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
@@ -39,6 +43,10 @@ config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@"
config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
+# Facebook T92898286
+config.llvm_test_bolt = lit.util.pythonize_bool("@LLVM_TEST_BOLT@")
+# End Facebook T92898286
+
# Plugins
lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@'
if lldb_build_intel_pt == '1':
diff --git a/lldb/test/Shell/helper/toolchain.py b/lldb/test/Shell/helper/toolchain.py
index 255955fc70d8c..7b7be06643166 100644
--- a/lldb/test/Shell/helper/toolchain.py
+++ b/lldb/test/Shell/helper/toolchain.py
@@ -165,6 +165,11 @@ def use_support_substitutions(config):
if config.cmake_sysroot:
host_flags += ["--sysroot={}".format(config.cmake_sysroot)]
+ # Facebook T92898286
+ if config.llvm_test_bolt:
+ host_flags += ["--post-link-optimize"]
+ # End Facebook T92898286
+
host_flags = " ".join(host_flags)
config.substitutions.append(("%clang_host", "%clang " + host_flags))
config.substitutions.append(("%clangxx_host", "%clangxx " + host_flags))
diff --git a/lldb/test/Shell/lit.site.cfg.py.in b/lldb/test/Shell/lit.site.cfg.py.in
index b69e7bce1bc0b..fe8323734b7db 100644
--- a/lldb/test/Shell/lit.site.cfg.py.in
+++ b/lldb/test/Shell/lit.site.cfg.py.in
@@ -1,5 +1,10 @@
@LIT_SITE_CFG_IN_HEADER@
+#Facebook T92898286
+import lit.util
+#End Facebook T92898286
+
+
config.llvm_src_root = "@LLVM_SOURCE_DIR@"
config.llvm_obj_root = "@LLVM_BINARY_DIR@"
config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
@@ -31,6 +36,10 @@ config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell")
+# Facebook T92898286
+config.llvm_test_bolt = lit.util.pythonize_bool("@LLVM_TEST_BOLT@")
+# End Facebook T92898286
+
import lit.llvm
lit.llvm.initialize(lit_config, config)
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 12618966c4adf..a08b477060f48 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -709,6 +709,10 @@ set(LLVM_LIB_FUZZING_ENGINE "" CACHE PATH
option(LLVM_USE_SPLIT_DWARF
"Use -gsplit-dwarf when compiling llvm and --gdb-index when linking." OFF)
+# Facebook T92898286
+option(LLVM_TEST_BOLT "Enable BOLT testing in non-BOLT tests that use clang" OFF)
+# End Facebook T92898286
+
# Define an option controlling whether we should build for 32-bit on 64-bit
# platforms, where supported.
if( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT (WIN32 OR ${CMAKE_SYSTEM_NAME} MATCHES "AIX"))
>From 5a0ff50f6ea5bc80d7051e345f658fde273ad39a Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Mon, 22 Jul 2024 12:31:51 -0700
Subject: [PATCH 2/4] [BOLT][DWARF][NFC] Revert "Split processUnitDIE into two
lambdas"
Summary: Reverts llvm/llvm-project#99225
Test Plan: No test plan, reverts changes
Reviewers: ayermolo, maks, #llvm-bolt
Reviewed By: ayermolo
Differential Revision: https://phabricator.intern.facebook.com/D60078320
Tasks: T187681160
---
bolt/lib/Rewrite/DWARFRewriter.cpp | 144 ++++++++++++++---------------
1 file changed, 67 insertions(+), 77 deletions(-)
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index ccb45f40c5c7a..1ec216b39e95c 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -620,10 +620,9 @@ void DWARFRewriter::updateDebugInfo() {
uint32_t CUIndex = 0;
std::mutex AccessMutex;
// Needs to be invoked in the same order as CUs are processed.
- llvm::DenseMap<uint64_t, uint64_t> LocListWritersIndexByCU;
- auto createRangeLocListAddressWriters = [&](DWARFUnit &CU) {
+ auto createRangeLocListAddressWriters =
+ [&](DWARFUnit &CU) -> DebugLocWriter * {
std::lock_guard<std::mutex> Lock(AccessMutex);
-
const uint16_t DwarfVersion = CU.getVersion();
if (DwarfVersion >= 5) {
auto AddrW = std::make_unique<DebugAddrWriterDwarf5>(
@@ -642,6 +641,7 @@ void DWARFRewriter::updateDebugInfo() {
RangeListsWritersByCU[*DWOId] = std::move(DWORangeListsSectionWriter);
}
AddressWritersByCU[CU.getOffset()] = std::move(AddrW);
+
} else {
auto AddrW =
std::make_unique<DebugAddrWriter>(&BC, CU.getAddressByteSize());
@@ -657,7 +657,7 @@ void DWARFRewriter::updateDebugInfo() {
std::move(LegacyRangesSectionWriterByCU);
}
}
- LocListWritersIndexByCU[CU.getOffset()] = CUIndex++;
+ return LocListWritersByCU[CUIndex++].get();
};
DWARF5AcceleratorTable DebugNamesTable(opts::CreateDebugNames, BC,
@@ -666,68 +666,74 @@ void DWARFRewriter::updateDebugInfo() {
DWPState State;
if (opts::WriteDWP)
initDWPState(State);
- auto processSplitCU = [&](DWARFUnit &Unit, DWARFUnit &SplitCU,
- DIEBuilder &DIEBlder,
- DebugRangesSectionWriter &TempRangesSectionWriter,
- DebugAddrWriter &AddressWriter) {
- DIEBuilder DWODIEBuilder(BC, &(SplitCU).getContext(), DebugNamesTable,
- &Unit);
- DWODIEBuilder.buildDWOUnit(SplitCU);
- std::string DWOName = "";
- std::optional<std::string> DwarfOutputPath =
- opts::DwarfOutputPath.empty()
- ? std::nullopt
- : std::optional<std::string>(opts::DwarfOutputPath.c_str());
- {
- std::lock_guard<std::mutex> Lock(AccessMutex);
- DWOName = DIEBlder.updateDWONameCompDir(
- *StrOffstsWriter, *StrWriter, Unit, DwarfOutputPath, std::nullopt);
- }
- DebugStrOffsetsWriter DWOStrOffstsWriter(BC);
- DebugStrWriter DWOStrWriter((SplitCU).getContext(), true);
- DWODIEBuilder.updateDWONameCompDirForTypes(
- DWOStrOffstsWriter, DWOStrWriter, SplitCU, DwarfOutputPath, DWOName);
- DebugLoclistWriter DebugLocDWoWriter(Unit, Unit.getVersion(), true,
- AddressWriter);
-
- updateUnitDebugInfo(SplitCU, DWODIEBuilder, DebugLocDWoWriter,
- TempRangesSectionWriter, AddressWriter);
- DebugLocDWoWriter.finalize(DWODIEBuilder,
- *DWODIEBuilder.getUnitDIEbyUnit(SplitCU));
- if (Unit.getVersion() >= 5)
- TempRangesSectionWriter.finalizeSection();
-
- emitDWOBuilder(DWOName, DWODIEBuilder, *this, SplitCU, Unit, State,
- DebugLocDWoWriter, DWOStrOffstsWriter, DWOStrWriter,
- GDBIndexSection);
- };
- auto processMainBinaryCU = [&](DWARFUnit &Unit, DIEBuilder &DIEBlder) {
- DebugAddrWriter &AddressWriter =
- *AddressWritersByCU[Unit.getOffset()].get();
- DebugRangesSectionWriter &RangesSectionWriter =
- Unit.getVersion() >= 5 ? *RangeListsSectionWriter.get()
- : *LegacyRangesSectionWriter.get();
- DebugLocWriter &DebugLocWriter =
- *LocListWritersByCU[LocListWritersIndexByCU[Unit.getOffset()]].get();
- std::optional<uint64_t> RangesBase;
+ auto processUnitDIE = [&](DWARFUnit *Unit, DIEBuilder *DIEBlder) {
+ // Check if the unit is a skeleton and we need special updates for it and
+ // its matching split/DWO CU.
std::optional<DWARFUnit *> SplitCU;
- std::optional<uint64_t> DWOId = Unit.getDWOId();
+ std::optional<uint64_t> RangesBase;
+ std::optional<uint64_t> DWOId = Unit->getDWOId();
if (DWOId)
SplitCU = BC.getDWOCU(*DWOId);
- if (Unit.getVersion() >= 5) {
- RangesBase = RangesSectionWriter.getSectionOffset() +
+ DebugLocWriter *DebugLocWriter = createRangeLocListAddressWriters(*Unit);
+ DebugRangesSectionWriter *RangesSectionWriter =
+ Unit->getVersion() >= 5 ? RangeListsSectionWriter.get()
+ : LegacyRangesSectionWriter.get();
+ DebugAddrWriter *AddressWriter =
+ AddressWritersByCU[Unit->getOffset()].get();
+ // Skipping CUs that failed to load.
+ if (SplitCU) {
+ DIEBuilder DWODIEBuilder(BC, &(*SplitCU)->getContext(), DebugNamesTable,
+ Unit);
+ DWODIEBuilder.buildDWOUnit(**SplitCU);
+ std::string DWOName = "";
+ std::optional<std::string> DwarfOutputPath =
+ opts::DwarfOutputPath.empty()
+ ? std::nullopt
+ : std::optional<std::string>(opts::DwarfOutputPath.c_str());
+ {
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ DWOName = DIEBlder->updateDWONameCompDir(
+ *StrOffstsWriter, *StrWriter, *Unit, DwarfOutputPath, std::nullopt);
+ }
+ DebugStrOffsetsWriter DWOStrOffstsWriter(BC);
+ DebugStrWriter DWOStrWriter((*SplitCU)->getContext(), true);
+ DWODIEBuilder.updateDWONameCompDirForTypes(DWOStrOffstsWriter,
+ DWOStrWriter, **SplitCU,
+ DwarfOutputPath, DWOName);
+ DebugLoclistWriter DebugLocDWoWriter(*Unit, Unit->getVersion(), true,
+ *AddressWriter);
+ DebugRangesSectionWriter *TempRangesSectionWriter = RangesSectionWriter;
+ if (Unit->getVersion() >= 5) {
+ TempRangesSectionWriter = RangeListsWritersByCU[*DWOId].get();
+ } else {
+ TempRangesSectionWriter = LegacyRangesWritersByCU[*DWOId].get();
+ RangesBase = RangesSectionWriter->getSectionOffset();
+ }
+
+ updateUnitDebugInfo(*(*SplitCU), DWODIEBuilder, DebugLocDWoWriter,
+ *TempRangesSectionWriter, *AddressWriter);
+ DebugLocDWoWriter.finalize(DWODIEBuilder,
+ *DWODIEBuilder.getUnitDIEbyUnit(**SplitCU));
+ if (Unit->getVersion() >= 5)
+ TempRangesSectionWriter->finalizeSection();
+
+ emitDWOBuilder(DWOName, DWODIEBuilder, *this, **SplitCU, *Unit, State,
+ DebugLocDWoWriter, DWOStrOffstsWriter, DWOStrWriter,
+ GDBIndexSection);
+ }
+
+ if (Unit->getVersion() >= 5) {
+ RangesBase = RangesSectionWriter->getSectionOffset() +
getDWARF5RngListLocListHeaderSize();
- RangesSectionWriter.initSection(Unit);
- StrOffstsWriter->finalizeSection(Unit, DIEBlder);
- } else if (SplitCU) {
- RangesBase = LegacyRangesSectionWriter.get()->getSectionOffset();
+ RangesSectionWriter->initSection(*Unit);
+ StrOffstsWriter->finalizeSection(*Unit, *DIEBlder);
}
- updateUnitDebugInfo(Unit, DIEBlder, DebugLocWriter, RangesSectionWriter,
- AddressWriter, RangesBase);
- DebugLocWriter.finalize(DIEBlder, *DIEBlder.getUnitDIEbyUnit(Unit));
- if (Unit.getVersion() >= 5)
- RangesSectionWriter.finalizeSection();
+ updateUnitDebugInfo(*Unit, *DIEBlder, *DebugLocWriter, *RangesSectionWriter,
+ *AddressWriter, RangesBase);
+ DebugLocWriter->finalize(*DIEBlder, *DIEBlder->getUnitDIEbyUnit(*Unit));
+ if (Unit->getVersion() >= 5)
+ RangesSectionWriter->finalizeSection();
};
DIEBuilder DIEBlder(BC, BC.DwCtx.get(), DebugNamesTable);
@@ -745,24 +751,8 @@ void DWARFRewriter::updateDebugInfo() {
CUPartitionVector PartVec = partitionCUs(*BC.DwCtx);
for (std::vector<DWARFUnit *> &Vec : PartVec) {
DIEBlder.buildCompileUnits(Vec);
- for (DWARFUnit *CU : DIEBlder.getProcessedCUs()) {
- createRangeLocListAddressWriters(*CU);
- std::optional<DWARFUnit *> SplitCU;
- std::optional<uint64_t> DWOId = CU->getDWOId();
- if (DWOId)
- SplitCU = BC.getDWOCU(*DWOId);
- if (!SplitCU)
- continue;
- DebugAddrWriter &AddressWriter =
- *AddressWritersByCU[CU->getOffset()].get();
- DebugRangesSectionWriter *TempRangesSectionWriter =
- CU->getVersion() >= 5 ? RangeListsWritersByCU[*DWOId].get()
- : LegacyRangesWritersByCU[*DWOId].get();
- processSplitCU(*CU, **SplitCU, DIEBlder, *TempRangesSectionWriter,
- AddressWriter);
- }
for (DWARFUnit *CU : DIEBlder.getProcessedCUs())
- processMainBinaryCU(*CU, DIEBlder);
+ processUnitDIE(CU, &DIEBlder);
finalizeCompileUnits(DIEBlder, *Streamer, OffsetMap,
DIEBlder.getProcessedCUs(), *FinalAddrWriter);
}
>From df0bd38c735ba8412cd2fd7b56f5c125c83e628f Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Mon, 22 Jul 2024 14:44:20 -0700
Subject: [PATCH 3/4] [BOLT][DWARF][NFC] Split processUnitDIE into two lambdas
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D60073712
---
bolt/lib/Rewrite/DWARFRewriter.cpp | 28 ++++++++++++++-----
...ypes-backward-forward-cross-reference.test | 10 +++++++
bolt/test/X86/dwarf5-locexpr-referrence.test | 16 +++++++++++
3 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 1ec216b39e95c..e40a0850b52e6 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -717,13 +717,27 @@ void DWARFRewriter::updateDebugInfo() {
if (Unit->getVersion() >= 5)
TempRangesSectionWriter->finalizeSection();
- emitDWOBuilder(DWOName, DWODIEBuilder, *this, **SplitCU, *Unit, State,
- DebugLocDWoWriter, DWOStrOffstsWriter, DWOStrWriter,
- GDBIndexSection);
- }
-
- if (Unit->getVersion() >= 5) {
- RangesBase = RangesSectionWriter->getSectionOffset() +
+ emitDWOBuilder(DWOName, DWODIEBuilder, *this, SplitCU, Unit, State,
+ DebugLocDWoWriter, DWOStrOffstsWriter, DWOStrWriter,
+ GDBIndexSection);
+ };
+ auto processMainBinaryCU = [&](DWARFUnit &Unit, DIEBuilder &DIEBlder) {
+ DebugAddrWriter &AddressWriter =
+ *AddressWritersByCU[Unit.getOffset()].get();
+ if (Unit.getVersion() >= 5)
+ RangeListsSectionWriter->setAddressWriter(&AddressWriter);
+ DebugRangesSectionWriter &RangesSectionWriter =
+ Unit.getVersion() >= 5 ? *RangeListsSectionWriter.get()
+ : *LegacyRangesSectionWriter.get();
+ DebugLocWriter &DebugLocWriter =
+ *LocListWritersByCU[LocListWritersIndexByCU[Unit.getOffset()]].get();
+ std::optional<uint64_t> RangesBase;
+ std::optional<DWARFUnit *> SplitCU;
+ std::optional<uint64_t> DWOId = Unit.getDWOId();
+ if (DWOId)
+ SplitCU = BC.getDWOCU(*DWOId);
+ if (Unit.getVersion() >= 5) {
+ RangesBase = RangesSectionWriter.getSectionOffset() +
getDWARF5RngListLocListHeaderSize();
RangesSectionWriter->initSection(*Unit);
StrOffstsWriter->finalizeSection(*Unit, *DIEBlder);
diff --git a/bolt/test/X86/dwarf5-dwarf4-types-backward-forward-cross-reference.test b/bolt/test/X86/dwarf5-dwarf4-types-backward-forward-cross-reference.test
index 070648c042c1d..576fe894b1113 100644
--- a/bolt/test/X86/dwarf5-dwarf4-types-backward-forward-cross-reference.test
+++ b/bolt/test/X86/dwarf5-dwarf4-types-backward-forward-cross-reference.test
@@ -5,6 +5,7 @@
# RUN: %clang %cflags %tmain.o %thelper.o -o %t.exe
# RUN: llvm-bolt %t.exe -o %t.bolt --update-debug-sections
# RUN: llvm-dwarfdump --show-form --verbose --debug-info %t.bolt | FileCheck --check-prefix=POSTCHECK %s
+# RUN: llvm-dwarfdump --show-form --verbose --debug-addr %t.bolt | FileCheck --check-prefix=POSTCHECKADDR %s
# RUN: llvm-dwarfdump --show-form --verbose --debug-types %t.bolt | FileCheck --check-prefix=POSTCHECKTU %s
## This test checks that BOLT handles correctly backward and forward cross CU references
@@ -29,6 +30,15 @@
# POSTCHECK: DW_TAG_variable [20]
# POSTCHECK: DW_AT_type [DW_FORM_ref_addr] (0x{{[0-9a-f]+}} "Foo3a")
+# POSTCHECKADDR: Addrs: [
+# POSTCHECKADDR-NEXT: 0x0000000000001360
+# POSTCHECKADDR-NEXT: 0x0000000000000000
+# POSTCHECKADDR-NEXT: ]
+# POSTCHECKADDR: Addrs: [
+# POSTCHECKADDR-NEXT: 0x00000000000013e0
+# POSTCHECKADDR-NEXT: 0x0000000000000000
+# POSTCHECKADDR-NEXT: ]
+
# POSTCHECKTU: version = 0x0004
# POSTCHECKTU: DW_TAG_type_unit
# POSTCHECKTU: DW_TAG_structure_type
diff --git a/bolt/test/X86/dwarf5-locexpr-referrence.test b/bolt/test/X86/dwarf5-locexpr-referrence.test
index ea73d7601b253..d55ff4b33ad08 100644
--- a/bolt/test/X86/dwarf5-locexpr-referrence.test
+++ b/bolt/test/X86/dwarf5-locexpr-referrence.test
@@ -5,6 +5,7 @@
# RUN: %clang %cflags -dwarf-5 %tmain.o %thelper.o -o %t.exe -Wl,-q
# RUN: llvm-bolt %t.exe -o %t.bolt --update-debug-sections
# RUN: llvm-dwarfdump --show-form --verbose --debug-info %t.bolt | FileCheck --check-prefix=CHECK %s
+# RUN: llvm-dwarfdump --show-form --verbose --debug-addr %t.bolt | FileCheck --check-prefix=CHECKADDR %s
## This test checks that we update relative DIE references with DW_OP_convert that are in locexpr.
@@ -19,3 +20,18 @@
# CHECK-SAME: DW_OP_convert (0x00000028 -> 0x00000092)
# CHECK-SAME: DW_OP_convert (0x0000002c -> 0x00000096)
# CHECK: version = 0x0005
+
+# CHECKADDR: Addrs: [
+# CHECKADDR-NEXT: 0x0000000000001330
+# CHECKADDR-NEXT: 0x0000000000000000
+# CHECKADDR-NEXT: 0x0000000000001333
+# CHECKADDR-NEXT: ]
+# CHECKADDR: Addrs: [
+# CHECKADDR-NEXT: 0x0000000000001340
+# CHECKADDR-NEXT: 0x0000000000000000
+# CHECKADDR-NEXT: 0x0000000000001343
+# CHECKADDR-NEXT: ]
+# CHECKADDR: Addrs: [
+# CHECKADDR-NEXT: 0x0000000000001320
+# CHECKADDR-NEXT: 0x0000000000000000
+# CHECKADDR-NEXT: ]
>From 006b0fde83b9d68d7e686eec494c5144b35e762d Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Mon, 22 Jul 2024 18:01:44 -0700
Subject: [PATCH 4/4] Formatting changes
---
bolt/lib/Rewrite/DWARFRewriter.cpp | 121 ++++++++++++++---------------
1 file changed, 59 insertions(+), 62 deletions(-)
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index e40a0850b52e6..f51863ed3f157 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -620,8 +620,8 @@ void DWARFRewriter::updateDebugInfo() {
uint32_t CUIndex = 0;
std::mutex AccessMutex;
// Needs to be invoked in the same order as CUs are processed.
- auto createRangeLocListAddressWriters =
- [&](DWARFUnit &CU) -> DebugLocWriter * {
+ llvm::DenseMap<uint64_t, uint64_t> LocListWritersIndexByCU;
+ auto createRangeLocListAddressWriters = [&](DWARFUnit &CU) {
std::lock_guard<std::mutex> Lock(AccessMutex);
const uint16_t DwarfVersion = CU.getVersion();
if (DwarfVersion >= 5) {
@@ -641,7 +641,6 @@ void DWARFRewriter::updateDebugInfo() {
RangeListsWritersByCU[*DWOId] = std::move(DWORangeListsSectionWriter);
}
AddressWritersByCU[CU.getOffset()] = std::move(AddrW);
-
} else {
auto AddrW =
std::make_unique<DebugAddrWriter>(&BC, CU.getAddressByteSize());
@@ -657,7 +656,7 @@ void DWARFRewriter::updateDebugInfo() {
std::move(LegacyRangesSectionWriterByCU);
}
}
- return LocListWritersByCU[CUIndex++].get();
+ LocListWritersIndexByCU[CU.getOffset()] = CUIndex++;
};
DWARF5AcceleratorTable DebugNamesTable(opts::CreateDebugNames, BC,
@@ -666,56 +665,36 @@ void DWARFRewriter::updateDebugInfo() {
DWPState State;
if (opts::WriteDWP)
initDWPState(State);
- auto processUnitDIE = [&](DWARFUnit *Unit, DIEBuilder *DIEBlder) {
- // Check if the unit is a skeleton and we need special updates for it and
- // its matching split/DWO CU.
- std::optional<DWARFUnit *> SplitCU;
- std::optional<uint64_t> RangesBase;
- std::optional<uint64_t> DWOId = Unit->getDWOId();
- if (DWOId)
- SplitCU = BC.getDWOCU(*DWOId);
- DebugLocWriter *DebugLocWriter = createRangeLocListAddressWriters(*Unit);
- DebugRangesSectionWriter *RangesSectionWriter =
- Unit->getVersion() >= 5 ? RangeListsSectionWriter.get()
- : LegacyRangesSectionWriter.get();
- DebugAddrWriter *AddressWriter =
- AddressWritersByCU[Unit->getOffset()].get();
- // Skipping CUs that failed to load.
- if (SplitCU) {
- DIEBuilder DWODIEBuilder(BC, &(*SplitCU)->getContext(), DebugNamesTable,
- Unit);
- DWODIEBuilder.buildDWOUnit(**SplitCU);
- std::string DWOName = "";
- std::optional<std::string> DwarfOutputPath =
- opts::DwarfOutputPath.empty()
- ? std::nullopt
- : std::optional<std::string>(opts::DwarfOutputPath.c_str());
- {
- std::lock_guard<std::mutex> Lock(AccessMutex);
- DWOName = DIEBlder->updateDWONameCompDir(
- *StrOffstsWriter, *StrWriter, *Unit, DwarfOutputPath, std::nullopt);
- }
- DebugStrOffsetsWriter DWOStrOffstsWriter(BC);
- DebugStrWriter DWOStrWriter((*SplitCU)->getContext(), true);
- DWODIEBuilder.updateDWONameCompDirForTypes(DWOStrOffstsWriter,
- DWOStrWriter, **SplitCU,
- DwarfOutputPath, DWOName);
- DebugLoclistWriter DebugLocDWoWriter(*Unit, Unit->getVersion(), true,
- *AddressWriter);
- DebugRangesSectionWriter *TempRangesSectionWriter = RangesSectionWriter;
- if (Unit->getVersion() >= 5) {
- TempRangesSectionWriter = RangeListsWritersByCU[*DWOId].get();
- } else {
- TempRangesSectionWriter = LegacyRangesWritersByCU[*DWOId].get();
- RangesBase = RangesSectionWriter->getSectionOffset();
- }
-
- updateUnitDebugInfo(*(*SplitCU), DWODIEBuilder, DebugLocDWoWriter,
- *TempRangesSectionWriter, *AddressWriter);
- DebugLocDWoWriter.finalize(DWODIEBuilder,
- *DWODIEBuilder.getUnitDIEbyUnit(**SplitCU));
- if (Unit->getVersion() >= 5)
- TempRangesSectionWriter->finalizeSection();
+ auto processSplitCU = [&](DWARFUnit &Unit, DWARFUnit &SplitCU,
+ DIEBuilder &DIEBlder,
+ DebugRangesSectionWriter &TempRangesSectionWriter,
+ DebugAddrWriter &AddressWriter) {
+ DIEBuilder DWODIEBuilder(BC, &(SplitCU).getContext(), DebugNamesTable,
+ &Unit);
+ DWODIEBuilder.buildDWOUnit(SplitCU);
+ std::string DWOName = "";
+ std::optional<std::string> DwarfOutputPath =
+ opts::DwarfOutputPath.empty()
+ ? std::nullopt
+ : std::optional<std::string>(opts::DwarfOutputPath.c_str());
+ {
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ DWOName = DIEBlder.updateDWONameCompDir(
+ *StrOffstsWriter, *StrWriter, Unit, DwarfOutputPath, std::nullopt);
+ }
+ DebugStrOffsetsWriter DWOStrOffstsWriter(BC);
+ DebugStrWriter DWOStrWriter((SplitCU).getContext(), true);
+ DWODIEBuilder.updateDWONameCompDirForTypes(
+ DWOStrOffstsWriter, DWOStrWriter, SplitCU, DwarfOutputPath, DWOName);
+ DebugLoclistWriter DebugLocDWoWriter(Unit, Unit.getVersion(), true,
+ AddressWriter);
+
+ updateUnitDebugInfo(SplitCU, DWODIEBuilder, DebugLocDWoWriter,
+ TempRangesSectionWriter, AddressWriter);
+ DebugLocDWoWriter.finalize(DWODIEBuilder,
+ *DWODIEBuilder.getUnitDIEbyUnit(SplitCU));
+ if (Unit.getVersion() >= 5)
+ TempRangesSectionWriter.finalizeSection();
emitDWOBuilder(DWOName, DWODIEBuilder, *this, SplitCU, Unit, State,
DebugLocDWoWriter, DWOStrOffstsWriter, DWOStrWriter,
@@ -739,15 +718,17 @@ void DWARFRewriter::updateDebugInfo() {
if (Unit.getVersion() >= 5) {
RangesBase = RangesSectionWriter.getSectionOffset() +
getDWARF5RngListLocListHeaderSize();
- RangesSectionWriter->initSection(*Unit);
- StrOffstsWriter->finalizeSection(*Unit, *DIEBlder);
+ RangesSectionWriter.initSection(Unit);
+ StrOffstsWriter->finalizeSection(Unit, DIEBlder);
+ } else if (SplitCU) {
+ RangesBase = LegacyRangesSectionWriter.get()->getSectionOffset();
}
- updateUnitDebugInfo(*Unit, *DIEBlder, *DebugLocWriter, *RangesSectionWriter,
- *AddressWriter, RangesBase);
- DebugLocWriter->finalize(*DIEBlder, *DIEBlder->getUnitDIEbyUnit(*Unit));
- if (Unit->getVersion() >= 5)
- RangesSectionWriter->finalizeSection();
+ updateUnitDebugInfo(Unit, DIEBlder, DebugLocWriter, RangesSectionWriter,
+ AddressWriter, RangesBase);
+ DebugLocWriter.finalize(DIEBlder, *DIEBlder.getUnitDIEbyUnit(Unit));
+ if (Unit.getVersion() >= 5)
+ RangesSectionWriter.finalizeSection();
};
DIEBuilder DIEBlder(BC, BC.DwCtx.get(), DebugNamesTable);
@@ -765,8 +746,24 @@ void DWARFRewriter::updateDebugInfo() {
CUPartitionVector PartVec = partitionCUs(*BC.DwCtx);
for (std::vector<DWARFUnit *> &Vec : PartVec) {
DIEBlder.buildCompileUnits(Vec);
+ for (DWARFUnit *CU : DIEBlder.getProcessedCUs()) {
+ createRangeLocListAddressWriters(*CU);
+ std::optional<DWARFUnit *> SplitCU;
+ std::optional<uint64_t> DWOId = CU->getDWOId();
+ if (DWOId)
+ SplitCU = BC.getDWOCU(*DWOId);
+ if (!SplitCU)
+ continue;
+ DebugAddrWriter &AddressWriter =
+ *AddressWritersByCU[CU->getOffset()].get();
+ DebugRangesSectionWriter *TempRangesSectionWriter =
+ CU->getVersion() >= 5 ? RangeListsWritersByCU[*DWOId].get()
+ : LegacyRangesWritersByCU[*DWOId].get();
+ processSplitCU(*CU, **SplitCU, DIEBlder, *TempRangesSectionWriter,
+ AddressWriter);
+ }
for (DWARFUnit *CU : DIEBlder.getProcessedCUs())
- processUnitDIE(CU, &DIEBlder);
+ processMainBinaryCU(*CU, DIEBlder);
finalizeCompileUnits(DIEBlder, *Streamer, OffsetMap,
DIEBlder.getProcessedCUs(), *FinalAddrWriter);
}
More information about the lldb-commits
mailing list