[Lldb-commits] [clang] [lldb] [llvm] Refactor GDB Index into a new file (PR #94405)
Sayhaan Siddiqui via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 4 15:10:26 PDT 2024
https://github.com/sayhaan created https://github.com/llvm/llvm-project/pull/94405
Create a new class and file for functions that update GDB index.
>From f3533c92b6b64f50a933a9eb2f9cc4229bbd8da3 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/7] 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 57f37c5023110f..7469db06839e3b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5465,6 +5465,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 b141e5f2adfab1..f7611af5763ab7 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 774c4eaf4d976b..619634578dfe60 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 39458dfc79afd2..2d53cd377f0330 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 d934349fe3ca3d..d4a62c51458cc1 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -248,6 +248,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 8b2d09ae41cd2a..602f45759e48f3 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 255955fc70d8c4..7b7be06643166d 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 b69e7bce1bc0be..fe8323734b7dbf 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 64898ab09772f4..c4ac19eaca2148 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 515bcbf85eb1fe60630a8d8e863e056a261a26ba Mon Sep 17 00:00:00 2001
From: Rafael Auler <rafaelauler at fb.com>
Date: Thu, 5 Aug 2021 14:17:07 -0700
Subject: [PATCH 2/7] Rebase: [Facebook] [MC] Introduce NeverAlign fragment
type
Summary:
Introduce NeverAlign fragment type.
The intended usage of this fragment is to insert it before a pair of
macro-op fusion eligible instructions. NeverAlign fragment ensures that
the next fragment (first instruction in the pair) does not end at a
given alignment boundary by emitting a minimal size nop if necessary.
In effect, it ensures that a pair of macro-fusible instructions is not
split by a given alignment boundary, which is a precondition for
macro-op fusion in modern Intel Cores (64B = cache line size, see Intel
Architecture Optimization Reference Manual, 2.3.2.1 Legacy Decode
Pipeline: Macro-Fusion).
This patch introduces functionality used by BOLT when emitting code with
MacroFusion alignment already in place.
The use case is different from BoundaryAlign and instruction bundling:
- BoundaryAlign can be extended to perform the desired alignment for the
first instruction in the macro-op fusion pair (D101817). However, this
approach has higher overhead due to reliance on relaxation as
BoundaryAlign requires in the general case - see
https://reviews.llvm.org/D97982#2710638.
- Instruction bundling: the intent of NeverAlign fragment is to prevent
the first instruction in a pair ending at a given alignment boundary, by
inserting at most one minimum size nop. It's OK if either instruction
crosses the cache line. Padding both instructions using bundles to not
cross the alignment boundary would result in excessive padding. There's
no straightforward way to request instruction bundling to avoid a given
end alignment for the first instruction in the bundle.
LLVM: https://reviews.llvm.org/D97982
Manual rebase conflict history:
https://phabricator.intern.facebook.com/D30142613
Test Plan: sandcastle
Reviewers: #llvm-bolt
Subscribers: phabricatorlinter
Differential Revision: https://phabricator.intern.facebook.com/D31361547
---
bolt/lib/Core/BinaryEmitter.cpp | 1 +
llvm/include/llvm/MC/MCFragment.h | 22 ++
llvm/include/llvm/MC/MCObjectStreamer.h | 2 +
llvm/include/llvm/MC/MCStreamer.h | 6 +
llvm/lib/MC/MCAssembler.cpp | 118 ++++++----
llvm/lib/MC/MCFragment.cpp | 12 +
llvm/lib/MC/MCObjectStreamer.cpp | 5 +
llvm/lib/MC/MCStreamer.cpp | 2 +
.../lib/Target/X86/AsmParser/X86AsmParser.cpp | 24 ++
llvm/test/MC/X86/directive-avoid_end_align.s | 208 ++++++++++++++++++
10 files changed, 363 insertions(+), 37 deletions(-)
create mode 100644 llvm/test/MC/X86/directive-avoid_end_align.s
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 0b44acb0816f2f..e09b9ff92de064 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -485,6 +485,7 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
// This assumes the second instruction in the macro-op pair will get
// assigned to its own MCRelaxableFragment. Since all JCC instructions
// are relaxable, we should be safe.
+ Streamer.emitNeverAlignCodeAtEnd(/*Alignment to avoid=*/64, *BC.STI);
}
if (!EmitCodeOnly) {
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
index a9b19dc56f16a5..256d98423e030c 100644
--- a/llvm/include/llvm/MC/MCFragment.h
+++ b/llvm/include/llvm/MC/MCFragment.h
@@ -33,6 +33,7 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
public:
enum FragmentType : uint8_t {
FT_Align,
+ FT_NeverAlign,
FT_Data,
FT_CompactEncodedInst,
FT_Fill,
@@ -344,6 +345,27 @@ class MCAlignFragment : public MCFragment {
}
};
+class MCNeverAlignFragment : public MCFragment {
+ /// The alignment the end of the next fragment should avoid.
+ unsigned Alignment;
+
+ /// When emitting Nops some subtargets have specific nop encodings.
+ const MCSubtargetInfo &STI;
+
+public:
+ MCNeverAlignFragment(unsigned Alignment, const MCSubtargetInfo &STI,
+ MCSection *Sec = nullptr)
+ : MCFragment(FT_NeverAlign, false, Sec), Alignment(Alignment), STI(STI) {}
+
+ unsigned getAlignment() const { return Alignment; }
+
+ const MCSubtargetInfo &getSubtargetInfo() const { return STI; }
+
+ static bool classof(const MCFragment *F) {
+ return F->getKind() == MCFragment::FT_NeverAlign;
+ }
+};
+
class MCFillFragment : public MCFragment {
uint8_t ValueSize;
/// Value to use for filling bytes.
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index e212d546139808..c7d760721e3691 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -157,6 +157,8 @@ class MCObjectStreamer : public MCStreamer {
unsigned MaxBytesToEmit = 0) override;
void emitCodeAlignment(Align ByteAlignment, const MCSubtargetInfo *STI,
unsigned MaxBytesToEmit = 0) override;
+ void emitNeverAlignCodeAtEnd(unsigned ByteAlignment,
+ const MCSubtargetInfo &STI) override;
void emitValueToOffset(const MCExpr *Offset, unsigned char Value,
SMLoc Loc) override;
void emitDwarfLocDirective(unsigned FileNo, unsigned Line, unsigned Column,
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index b7468cf70a6643..dd813192d9ca09 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -887,6 +887,12 @@ class MCStreamer {
virtual void emitCodeAlignment(Align Alignment, const MCSubtargetInfo *STI,
unsigned MaxBytesToEmit = 0);
+ /// If the end of the fragment following this NeverAlign fragment ever gets
+ /// aligned to \p ByteAlignment, this fragment emits a single nop before the
+ /// following fragment to break this end-alignment.
+ virtual void emitNeverAlignCodeAtEnd(unsigned ByteAlignment,
+ const MCSubtargetInfo &STI);
+
/// Emit some number of copies of \p Value until the byte offset \p
/// Offset is reached.
///
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index ad30b5ce9e6314..62baeb93ea7d07 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -298,6 +298,43 @@ bool MCAssembler::evaluateFixup(const MCAsmLayout &Layout, const MCFixup &Fixup,
return IsResolved;
}
+/// Check if the branch crosses the boundary.
+///
+/// \param StartAddr start address of the fused/unfused branch.
+/// \param Size size of the fused/unfused branch.
+/// \param BoundaryAlignment alignment requirement of the branch.
+/// \returns true if the branch cross the boundary.
+static bool mayCrossBoundary(uint64_t StartAddr, uint64_t Size,
+ Align BoundaryAlignment) {
+ uint64_t EndAddr = StartAddr + Size;
+ return (StartAddr >> Log2(BoundaryAlignment)) !=
+ ((EndAddr - 1) >> Log2(BoundaryAlignment));
+}
+
+/// Check if the branch is against the boundary.
+///
+/// \param StartAddr start address of the fused/unfused branch.
+/// \param Size size of the fused/unfused branch.
+/// \param BoundaryAlignment alignment requirement of the branch.
+/// \returns true if the branch is against the boundary.
+static bool isAgainstBoundary(uint64_t StartAddr, uint64_t Size,
+ Align BoundaryAlignment) {
+ uint64_t EndAddr = StartAddr + Size;
+ return (EndAddr & (BoundaryAlignment.value() - 1)) == 0;
+}
+
+/// Check if the branch needs padding.
+///
+/// \param StartAddr start address of the fused/unfused branch.
+/// \param Size size of the fused/unfused branch.
+/// \param BoundaryAlignment alignment requirement of the branch.
+/// \returns true if the branch needs padding.
+static bool needPadding(uint64_t StartAddr, uint64_t Size,
+ Align BoundaryAlignment) {
+ return mayCrossBoundary(StartAddr, Size, BoundaryAlignment) ||
+ isAgainstBoundary(StartAddr, Size, BoundaryAlignment);
+}
+
uint64_t MCAssembler::computeFragmentSize(const MCAsmLayout &Layout,
const MCFragment &F) const {
assert(getBackendPtr() && "Requires assembler backend");
@@ -358,6 +395,41 @@ uint64_t MCAssembler::computeFragmentSize(const MCAsmLayout &Layout,
return Size;
}
+ case MCFragment::FT_NeverAlign: {
+ // Disclaimer: NeverAlign fragment size depends on the size of its immediate
+ // successor, but NeverAlign need not be a MCRelaxableFragment.
+ // NeverAlign fragment size is recomputed if the successor is relaxed:
+ // - If RelaxableFragment is relaxed, it gets invalidated by marking its
+ // predecessor as LastValidFragment.
+ // - This forces the assembler to call MCAsmLayout::layoutFragment on that
+ // relaxable fragment, which in turn will always ask the predecessor to
+ // compute its size (see "computeFragmentSize(prev)" in layoutFragment).
+ //
+ // In short, the simplest way to ensure that computeFragmentSize() is sane
+ // is to establish the following rule: it should never examine fragments
+ // after the current fragment in the section. If we logically need to
+ // examine any fragment after the current fragment, we need to do that using
+ // relaxation, inside MCAssembler::layoutSectionOnce.
+ const MCNeverAlignFragment &NAF = cast<MCNeverAlignFragment>(F);
+ const MCFragment *NF = F.getNextNode();
+ uint64_t Offset = Layout.getFragmentOffset(&NAF);
+ size_t NextFragSize = 0;
+ if (const auto *NextFrag = dyn_cast<MCRelaxableFragment>(NF)) {
+ NextFragSize = NextFrag->getContents().size();
+ } else if (const auto *NextFrag = dyn_cast<MCDataFragment>(NF)) {
+ NextFragSize = NextFrag->getContents().size();
+ } else {
+ llvm_unreachable("Didn't find the expected fragment after NeverAlign");
+ }
+ // Check if the next fragment ends at the alignment we want to avoid.
+ if (isAgainstBoundary(Offset, NextFragSize, Align(NAF.getAlignment()))) {
+ // Avoid this alignment by introducing minimum nop.
+ assert(getBackend().getMinimumNopSize() != NAF.getAlignment());
+ return getBackend().getMinimumNopSize();
+ }
+ return 0;
+ }
+
case MCFragment::FT_Org: {
const MCOrgFragment &OF = cast<MCOrgFragment>(F);
MCValue Value;
@@ -581,6 +653,15 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
break;
}
+ case MCFragment::FT_NeverAlign: {
+ const MCNeverAlignFragment &NAF = cast<MCNeverAlignFragment>(F);
+ if (!Asm.getBackend().writeNopData(OS, FragmentSize,
+ &NAF.getSubtargetInfo()))
+ report_fatal_error("unable to write nop sequence of " +
+ Twine(FragmentSize) + " bytes");
+ break;
+ }
+
case MCFragment::FT_Data:
++stats::EmittedDataFragments;
OS << cast<MCDataFragment>(F).getContents();
@@ -1052,43 +1133,6 @@ bool MCAssembler::relaxLEB(MCAsmLayout &Layout, MCLEBFragment &LF) {
return OldSize != LF.getContents().size();
}
-/// Check if the branch crosses the boundary.
-///
-/// \param StartAddr start address of the fused/unfused branch.
-/// \param Size size of the fused/unfused branch.
-/// \param BoundaryAlignment alignment requirement of the branch.
-/// \returns true if the branch cross the boundary.
-static bool mayCrossBoundary(uint64_t StartAddr, uint64_t Size,
- Align BoundaryAlignment) {
- uint64_t EndAddr = StartAddr + Size;
- return (StartAddr >> Log2(BoundaryAlignment)) !=
- ((EndAddr - 1) >> Log2(BoundaryAlignment));
-}
-
-/// Check if the branch is against the boundary.
-///
-/// \param StartAddr start address of the fused/unfused branch.
-/// \param Size size of the fused/unfused branch.
-/// \param BoundaryAlignment alignment requirement of the branch.
-/// \returns true if the branch is against the boundary.
-static bool isAgainstBoundary(uint64_t StartAddr, uint64_t Size,
- Align BoundaryAlignment) {
- uint64_t EndAddr = StartAddr + Size;
- return (EndAddr & (BoundaryAlignment.value() - 1)) == 0;
-}
-
-/// Check if the branch needs padding.
-///
-/// \param StartAddr start address of the fused/unfused branch.
-/// \param Size size of the fused/unfused branch.
-/// \param BoundaryAlignment alignment requirement of the branch.
-/// \returns true if the branch needs padding.
-static bool needPadding(uint64_t StartAddr, uint64_t Size,
- Align BoundaryAlignment) {
- return mayCrossBoundary(StartAddr, Size, BoundaryAlignment) ||
- isAgainstBoundary(StartAddr, Size, BoundaryAlignment);
-}
-
bool MCAssembler::relaxBoundaryAlign(MCAsmLayout &Layout,
MCBoundaryAlignFragment &BF) {
// BoundaryAlignFragment that doesn't need to align any fragment should not be
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index a8da46dbd87279..2626da7e0391a6 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -274,6 +274,9 @@ void MCFragment::destroy() {
case FT_Align:
delete cast<MCAlignFragment>(this);
return;
+ case FT_NeverAlign:
+ delete cast<MCNeverAlignFragment>(this);
+ return;
case FT_Data:
delete cast<MCDataFragment>(this);
return;
@@ -342,6 +345,9 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
OS << "<";
switch (getKind()) {
case MCFragment::FT_Align: OS << "MCAlignFragment"; break;
+ case MCFragment::FT_NeverAlign:
+ OS << "MCNeverAlignFragment";
+ break;
case MCFragment::FT_Data: OS << "MCDataFragment"; break;
case MCFragment::FT_CompactEncodedInst:
OS << "MCCompactEncodedInstFragment"; break;
@@ -381,6 +387,12 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
<< " MaxBytesToEmit:" << AF->getMaxBytesToEmit() << ">";
break;
}
+ case MCFragment::FT_NeverAlign: {
+ const MCNeverAlignFragment *NAF = cast<MCNeverAlignFragment>(this);
+ OS << "\n ";
+ OS << " Alignment:" << NAF->getAlignment() << ">";
+ break;
+ }
case MCFragment::FT_Data: {
const auto *DF = cast<MCDataFragment>(this);
OS << "\n ";
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 0ccade91677a41..117475b7dd90ba 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -658,6 +658,11 @@ void MCObjectStreamer::emitCodeAlignment(Align Alignment,
cast<MCAlignFragment>(getCurrentFragment())->setEmitNops(true, STI);
}
+void MCObjectStreamer::emitNeverAlignCodeAtEnd(unsigned ByteAlignment,
+ const MCSubtargetInfo &STI) {
+ insert(new MCNeverAlignFragment(ByteAlignment, STI));
+}
+
void MCObjectStreamer::emitValueToOffset(const MCExpr *Offset,
unsigned char Value,
SMLoc Loc) {
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index 199d865ea3496d..a97cba6c89972f 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1235,6 +1235,8 @@ void MCStreamer::emitValueToAlignment(Align Alignment, int64_t Value,
unsigned MaxBytesToEmit) {}
void MCStreamer::emitCodeAlignment(Align Alignment, const MCSubtargetInfo *STI,
unsigned MaxBytesToEmit) {}
+void MCStreamer::emitNeverAlignCodeAtEnd(unsigned ByteAlignment,
+ const MCSubtargetInfo &STI) {}
void MCStreamer::emitValueToOffset(const MCExpr *Offset, unsigned char Value,
SMLoc Loc) {}
void MCStreamer::emitBundleAlignMode(Align Alignment) {}
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 6623106109316b..6c6bd2cf31e864 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -1153,6 +1153,7 @@ class X86AsmParser : public MCTargetAsmParser {
bool parseDirectiveArch();
bool parseDirectiveNops(SMLoc L);
bool parseDirectiveEven(SMLoc L);
+ bool parseDirectiveAvoidEndAlign(SMLoc L);
bool ParseDirectiveCode(StringRef IDVal, SMLoc L);
/// CodeView FPO data directives.
@@ -4601,6 +4602,8 @@ bool X86AsmParser::ParseDirective(AsmToken DirectiveID) {
return false;
} else if (IDVal == ".nops")
return parseDirectiveNops(DirectiveID.getLoc());
+ else if (IDVal == ".avoid_end_align")
+ return parseDirectiveAvoidEndAlign(DirectiveID.getLoc());
else if (IDVal == ".even")
return parseDirectiveEven(DirectiveID.getLoc());
else if (IDVal == ".cv_fpo_proc")
@@ -4695,6 +4698,27 @@ bool X86AsmParser::parseDirectiveEven(SMLoc L) {
return false;
}
+/// Directive for NeverAlign fragment testing, not for general usage!
+/// parseDirectiveAvoidEndAlign
+/// ::= .avoid_end_align alignment
+bool X86AsmParser::parseDirectiveAvoidEndAlign(SMLoc L) {
+ int64_t Alignment = 0;
+ SMLoc AlignmentLoc;
+ AlignmentLoc = getTok().getLoc();
+ if (getParser().checkForValidSection() ||
+ getParser().parseAbsoluteExpression(Alignment))
+ return true;
+
+ if (getParser().parseEOL("unexpected token in directive"))
+ return true;
+
+ if (Alignment <= 0)
+ return Error(AlignmentLoc, "expected a positive alignment");
+
+ getParser().getStreamer().emitNeverAlignCodeAtEnd(Alignment, getSTI());
+ return false;
+}
+
/// ParseDirectiveCode
/// ::= .code16 | .code32 | .code64
bool X86AsmParser::ParseDirectiveCode(StringRef IDVal, SMLoc L) {
diff --git a/llvm/test/MC/X86/directive-avoid_end_align.s b/llvm/test/MC/X86/directive-avoid_end_align.s
new file mode 100644
index 00000000000000..1d748401edc12e
--- /dev/null
+++ b/llvm/test/MC/X86/directive-avoid_end_align.s
@@ -0,0 +1,208 @@
+# RUN: llvm-mc -triple=x86_64 -filetype=obj %s | llvm-objdump --no-show-raw-insn -d - | FileCheck %s
+# RUN: not llvm-mc -triple=x86_64 --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
+
+# avoid_end_align has no effect since test doesn't end at alignment boundary:
+.avoid_end_align 64
+# CHECK-NOT: nop
+ testl %eax, %eax
+# CHECK: testl %eax, %eax
+ je .LBB0
+
+.fill 58, 1, 0x00
+# NeverAlign followed by MCDataFragment:
+# avoid_end_align inserts nop because `test` would end at alignment boundary:
+.avoid_end_align 64
+# CHECK: 3e: nop
+ testl %eax, %eax
+# CHECK-NEXT: 3f: testl %eax, %eax
+ je .LBB0
+# CHECK-NEXT: 41: je
+.LBB0:
+ retq
+
+.p2align 6
+.L0:
+.nops 57
+ int3
+# NeverAlign followed by RelaxableFragment:
+.avoid_end_align 64
+# CHECK: ba: nop
+ cmpl $(.L1-.L0), %eax
+# CHECK-NEXT: bb: cmpl
+ je .L0
+# CHECK-NEXT: c1: je
+.nops 65
+.L1:
+
+###############################################################################
+# Experiment A:
+# Check that NeverAlign doesn't introduce infinite loops in layout.
+# Control:
+# 1. NeverAlign fragment is not added,
+# 2. Short formats of cmp and jcc are used (3 and 2 bytes respectively),
+# 3. cmp and jcc are placed such that to be split by 64B alignment boundary.
+# 4. jcc would be relaxed to a longer format if at least one byte is added
+# between .L10 and je itself, e.g. by adding a NeverAlign padding byte,
+# or relaxing cmp instruction.
+# 5. cmp would be relaxed to a longer format if at least one byte is added
+# between .L11 and .L12, e.g. due to relaxing jcc instruction.
+.p2align 6
+# CHECK: 140: int3
+.fill 2, 1, 0xcc
+.L10:
+.nops 122
+ int3
+# CHECK: 1bc: int3
+# no avoid_end_align here
+# CHECK-NOT: nop
+ cmp $(.L12-.L11), %eax
+# CHECK: 1bd: cmpl
+.L11:
+ je .L10
+# CHECK-NEXT: 1c0: je
+.nops 125
+.L12:
+
+# Experiment:
+# Same setup as control, except NeverAlign fragment is added before cmp.
+# Expected effect:
+# 1. NeverAlign pads cmp+jcc by one byte since cmp and jcc are split by a 64B
+# alignment boundary,
+# 2. This extra byte forces jcc relaxation to a longer format (Control rule #4),
+# 3. This results in an cmp relaxation (Control rule #5),
+# 4. Which in turn makes NeverAlign fragment unnecessary as cmp and jcc
+# are no longer split by an alignment boundary (cmp crosses the boundary).
+# 5. NeverAlign padding is removed.
+# 6. cmp and jcc instruction remain in relaxed form.
+# 7. Relaxation converges, layout succeeds.
+.p2align 6
+# CHECK: 240: int3
+.fill 2, 1, 0xcc
+.L20:
+.nops 122
+ int3
+# CHECK: 2bc: int3
+.avoid_end_align 64
+# CHECK-NOT: nop
+ cmp $(.L22-.L21), %eax
+# CHECK-NEXT: 2bd: cmpl
+.L21:
+ je .L20
+# CHECK-NEXT: 2c3: je
+.nops 125
+.L22:
+
+###############################################################################
+# Experiment B: similar to exp A, but we check that once NeverAlign padding is
+# removed from the layout (exp A, experiment step 5), the increased distance
+# between the symbols L33 and L34 triggers the relaxation of instruction at
+# label L32.
+#
+# Control 1: using a one-byte instruction at L33 (site of NeverAlign) leads to
+# steps 2-3 of exp A, experiment:
+# 2. This extra byte forces jcc relaxation to a longer format (Control rule #4),
+# 3. This results in an cmp relaxation (Control rule #5),
+# => short cmp under L32
+.p2align 6
+# CHECK: 380: int3
+.fill 2, 1, 0xcc
+.L30:
+.nops 122
+ int3
+# CHECK: 3fc: int3
+ hlt
+#.avoid_end_align 64
+.L33:
+ cmp $(.L32-.L31), %eax
+# CHECK: 3fe: cmpl
+.L31:
+ je .L30
+# CHECK-NEXT: 404: je
+.nops 114
+.p2align 1
+ int3
+ int3
+# CHECK: 47c: int3
+.L34:
+.nops 9
+.L32:
+ cmp $(.L33-.L34), %eax
+# CHECK: 487: cmp
+# note that the size of cmp is 48a-487 == 3 bytes (distance is exactly -128)
+ int3
+# CHECK-NEXT: 48a: int3
+
+# Control 2: leaving out a byte at L43 (site of NeverAlign), plus
+# relaxed jcc and cmp leads to a relaxed cmp under L42 (-129 as cmp's immediate)
+.p2align 6
+# CHECK: 4c0: int3
+.fill 2, 1, 0xcc
+.L40:
+.nops 122
+ int3
+# CHECK: 53c: int3
+# int3
+#.avoid_end_align 64
+.L43:
+ cmp $(.L42-.L41+0x100), %eax
+# CHECK: 53d: cmpl
+.L41:
+ je .L40+0x100
+# CHECK-NEXT: 543: je
+.nops 114
+.p2align 1
+ int3
+ int3
+# CHECK: 5bc: int3
+.L44:
+.nops 9
+.L42:
+ cmp $(.L43-.L44), %eax
+# CHECK: 5c7: cmp
+# note that the size of cmp is 5cd-5c7 == 6 bytes (distance is exactly -129)
+ int3
+# CHECK-NEXT: 5cd: int3
+
+# Experiment
+# Checking if removing NeverAlign padding at L53 as a result of alignment and
+# relaxation of cmp and jcc following it (see exp A), thus reproducing the case
+# in Control 2 (getting a relaxed cmp under L52), is handled correctly.
+.p2align 6
+# CHECK: 600: int3
+.fill 2, 1, 0xcc
+.L50:
+.nops 122
+ int3
+# CHECK: 67c: int3
+.avoid_end_align 64
+.L53:
+# CHECK-NOT: nop
+ cmp $(.L52-.L51), %eax
+# CHECK-NEXT: 67d: cmpl
+.L51:
+ je .L50
+# CHECK-NEXT: 683: je
+.nops 114
+.p2align 1
+ int3
+ int3
+# CHECK: 6fc: int3
+.L54:
+.nops 9
+.L52:
+ cmp $(.L53-.L54), %eax
+# CHECK: 707: cmp
+# note that the size of cmp is 70d-707 == 6 bytes (distance is exactly -129)
+ int3
+# CHECK-NEXT: 70d: int3
+
+.ifdef ERR
+# ERR: {{.*}}.s:[[#@LINE+1]]:17: error: unknown token in expression
+.avoid_end_align
+# ERR: {{.*}}.s:[[#@LINE+1]]:18: error: expected absolute expression
+.avoid_end_align x
+# ERR: {{.*}}.s:[[#@LINE+1]]:18: error: expected a positive alignment
+.avoid_end_align 0
+# ERR: {{.*}}.s:[[#@LINE+1]]:20: error: unexpected token in directive
+.avoid_end_align 64, 0
+.endif
>From a5a5a85f69d5b0ae337161b263b47115c783c44b Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at meta.com>
Date: Tue, 16 Apr 2024 14:48:55 -0700
Subject: [PATCH 3/7] [BOLT] Fix ValidateMemRefs
Summary:
In ValidateMemRefs pass, when we validate references of the form
`Symbol + Addend`, we should check Symbol against aliasing a jump table
instead of the Symbol + Addend value.
https://github.com/llvm/llvm-project/pull/88838
Test Plan: NFC
Reviewers: aaupov, #llvm-bolt
Reviewed By: aaupov
Differential Revision: https://phabricator.intern.facebook.com/D56213679
Tags: accept2ship
---
bolt/lib/Passes/ValidateMemRefs.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/bolt/lib/Passes/ValidateMemRefs.cpp b/bolt/lib/Passes/ValidateMemRefs.cpp
index f29a97c43f497c..1d2c230fa7106a 100644
--- a/bolt/lib/Passes/ValidateMemRefs.cpp
+++ b/bolt/lib/Passes/ValidateMemRefs.cpp
@@ -29,8 +29,7 @@ bool ValidateMemRefs::checkAndFixJTReference(BinaryFunction &BF, MCInst &Inst,
if (!BD)
return false;
- const uint64_t TargetAddress = BD->getAddress() + Offset;
- JumpTable *JT = BC.getJumpTableContainingAddress(TargetAddress);
+ JumpTable *JT = BC.getJumpTableContainingAddress(BD->getAddress());
if (!JT)
return false;
@@ -41,10 +40,10 @@ bool ValidateMemRefs::checkAndFixJTReference(BinaryFunction &BF, MCInst &Inst,
// Accessing a jump table in another function. This is not a
// legitimate jump table access, we need to replace the reference to
// the jump table label with a regular rodata reference. Get a
- // non-JT reference by fetching the symbol 1 byte before the JT
- // label.
- MCSymbol *NewSym = BC.getOrCreateGlobalSymbol(TargetAddress - 1, "DATAat");
- BC.MIB->setOperandToSymbolRef(Inst, OperandNum, NewSym, 1, &*BC.Ctx, 0);
+ // non-JT reference by fetching the symbol 1 byte before the JT label.
+ MCSymbol *NewSym = BC.getOrCreateGlobalSymbol(BD->getAddress() - 1, "DATAat");
+ BC.MIB->setOperandToSymbolRef(Inst, OperandNum, NewSym, Offset + 1, &*BC.Ctx,
+ 0);
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: replaced reference @" << BF.getPrintName()
<< " from " << BD->getName() << " to " << NewSym->getName()
<< " + 1\n");
>From 7bfe0e4f67e437894b4c653e57f7f69549a0cc82 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at meta.com>
Date: Sun, 2 Jun 2024 11:29:35 -0700
Subject: [PATCH 4/7] Revert "[MC] Speed up AttemptToFoldSymbolOffsetDifference
in the absence of MCAsmLayout"
Summary:
This reverts commit 92d2c1b67835248d5d7371fa06381f970687c68f.
Causes timeouts in BOLT tests
Test Plan: sandcastle
Reviewers: ayermolo, davidino, #llvm-bolt
Reviewed By: davidino
Differential Revision: https://phabricator.intern.facebook.com/D58066252
Tasks: T191101004
Tags: accept2ship, publish_when_ready
---
llvm/lib/MC/MCExpr.cpp | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index b065d03651c452..9add574b0e5b83 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -675,14 +675,8 @@ static void AttemptToFoldSymbolOffsetDifference(
if (FA == FB) {
Reverse = SA.getOffset() < SB.getOffset();
} else if (!isa<MCDummyFragment>(FA)) {
- // Testing FA < FB is slow. Use setLayoutOrder to speed up computation.
- // The formal layout order will be finalized in MCAssembler::layout.
- if (FA->getLayoutOrder() == 0 || FB->getLayoutOrder()== 0) {
- unsigned LayoutOrder = 0;
- for (MCFragment &F : *FA->getParent())
- F.setLayoutOrder(++LayoutOrder);
- }
- Reverse = FA->getLayoutOrder() < FB->getLayoutOrder();
+ Reverse = std::find_if(std::next(FA->getIterator()), SecA.end(),
+ [&](auto &I) { return &I == FB; }) != SecA.end();
}
uint64_t SAOffset = SA.getOffset(), SBOffset = SB.getOffset();
>From c96444c01dc911d9ed66233567c66c6c2c159eba Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at meta.com>
Date: Fri, 31 May 2024 11:09:31 -0700
Subject: [PATCH 5/7] [MC] Speed up AttemptToFoldSymbolOffsetDifference in the
absence of MCAsmLayout
Summary:
The `FA < FB` check added by https://reviews.llvm.org/D153096 is slow.
Compute an informal layout order to speed up computation when
`AttemptToFoldSymbolOffsetDifference` is repeatedly called for the same
section.
Commit 9500a5d02e23f9b43294e5f662ac099f8989c0e4 ("[MC] Make UseAssemblerInfoForParsing mostly true")
exposed this performance pitfall, which was mitigated by
`setUseAssemblerInfoForParsing(false)` workarounds (e.g. commit
245491a9f384e4c53421196533c2a2b693efaf8d). The workaround can be removed
now.
Test Plan: sandcastle
Reviewers: rafaelauler, davidino, #llvm-bolt
Reviewed By: davidino
Differential Revision: https://phabricator.intern.facebook.com/D58066251
Tasks: T191101004
Tags: accept2ship, publish_when_ready
---
llvm/lib/MC/MCExpr.cpp | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index 9add574b0e5b83..b065d03651c452 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -675,8 +675,14 @@ static void AttemptToFoldSymbolOffsetDifference(
if (FA == FB) {
Reverse = SA.getOffset() < SB.getOffset();
} else if (!isa<MCDummyFragment>(FA)) {
- Reverse = std::find_if(std::next(FA->getIterator()), SecA.end(),
- [&](auto &I) { return &I == FB; }) != SecA.end();
+ // Testing FA < FB is slow. Use setLayoutOrder to speed up computation.
+ // The formal layout order will be finalized in MCAssembler::layout.
+ if (FA->getLayoutOrder() == 0 || FB->getLayoutOrder()== 0) {
+ unsigned LayoutOrder = 0;
+ for (MCFragment &F : *FA->getParent())
+ F.setLayoutOrder(++LayoutOrder);
+ }
+ Reverse = FA->getLayoutOrder() < FB->getLayoutOrder();
}
uint64_t SAOffset = SA.getOffset(), SBOffset = SB.getOffset();
>From 534606673a532da8e176726babfdb657c9dd80b0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at meta.com>
Date: Sun, 2 Jun 2024 11:29:35 -0700
Subject: [PATCH 6/7] Revert "[MC] Speed up AttemptToFoldSymbolOffsetDifference
in the absence of MCAsmLayout"
Summary:
This reverts commit 92d2c1b67835248d5d7371fa06381f970687c68f.
Causes timeouts in BOLT tests
Test Plan: sandcastle
Reviewers: ayermolo, davidino, #llvm-bolt
Reviewed By: davidino
Differential Revision: https://phabricator.intern.facebook.com/D58066252
Tasks: T191101004
Tags: accept2ship, publish_when_ready
---
llvm/lib/MC/MCExpr.cpp | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index b065d03651c452..9add574b0e5b83 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -675,14 +675,8 @@ static void AttemptToFoldSymbolOffsetDifference(
if (FA == FB) {
Reverse = SA.getOffset() < SB.getOffset();
} else if (!isa<MCDummyFragment>(FA)) {
- // Testing FA < FB is slow. Use setLayoutOrder to speed up computation.
- // The formal layout order will be finalized in MCAssembler::layout.
- if (FA->getLayoutOrder() == 0 || FB->getLayoutOrder()== 0) {
- unsigned LayoutOrder = 0;
- for (MCFragment &F : *FA->getParent())
- F.setLayoutOrder(++LayoutOrder);
- }
- Reverse = FA->getLayoutOrder() < FB->getLayoutOrder();
+ Reverse = std::find_if(std::next(FA->getIterator()), SecA.end(),
+ [&](auto &I) { return &I == FB; }) != SecA.end();
}
uint64_t SAOffset = SA.getOffset(), SBOffset = SB.getOffset();
>From 37c9d347e9735fa563ac3ce933923949bae3caf1 Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Tue, 4 Jun 2024 13:03:53 -0700
Subject: [PATCH 7/7] Refactor GDBIndex into new file
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D58156977
---
bolt/include/bolt/Core/GDBIndex.h | 60 ++++++++++
bolt/lib/Core/CMakeLists.txt | 1 +
bolt/lib/Core/GDBIndex.cpp | 182 ++++++++++++++++++++++++++++++
3 files changed, 243 insertions(+)
create mode 100644 bolt/include/bolt/Core/GDBIndex.h
create mode 100644 bolt/lib/Core/GDBIndex.cpp
diff --git a/bolt/include/bolt/Core/GDBIndex.h b/bolt/include/bolt/Core/GDBIndex.h
new file mode 100644
index 00000000000000..0ea588e9cba465
--- /dev/null
+++ b/bolt/include/bolt/Core/GDBIndex.h
@@ -0,0 +1,60 @@
+//===-- bolt/Core/GDBIndex.h - GDB Index support -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// This file contains declaration of classes required for generation of
+/// .gdb_index section.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef BOLT_CORE_GDB_INDEX_H
+#define BOLT_CORE_GDB_INDEX_H
+
+#include "bolt/Core/BinaryContext.h"
+#include <vector>
+
+namespace llvm {
+namespace bolt {
+
+class GDBIndex {
+public:
+ /// Contains information about TU so we can write out correct entries in GDB
+ /// index.
+ struct GDBIndexTUEntry {
+ uint64_t UnitOffset;
+ uint64_t TypeHash;
+ uint64_t TypeDIERelativeOffset;
+ };
+
+private:
+ BinaryContext &BC;
+
+ /// Entries for GDB Index Types CU List
+ using GDBIndexTUEntryType = std::vector<GDBIndexTUEntry>;
+ GDBIndexTUEntryType GDBIndexTUEntryVector;
+
+public:
+ GDBIndex(BinaryContext &BC) : BC(BC) {}
+
+ /// Adds an GDBIndexTUEntry if .gdb_index section exists.
+ void addGDBTypeUnitEntry(const GDBIndexTUEntry &Entry);
+
+ /// Rewrite .gdb_index section if present.
+ void updateGdbIndexSection(
+ CUOffsetMap &CUMap, uint32_t NumCUs,
+ std::unique_ptr<DebugARangesSectionWriter> &ARangesSectionWriter);
+
+ /// Returns all entries needed for Types CU list
+ const GDBIndexTUEntryType &getGDBIndexTUEntryVector() const {
+ return GDBIndexTUEntryVector;
+ }
+};
+
+} // namespace bolt
+} // namespace llvm
+
+#endif
diff --git a/bolt/lib/Core/CMakeLists.txt b/bolt/lib/Core/CMakeLists.txt
index 441df9fe084648..873cf67a56291f 100644
--- a/bolt/lib/Core/CMakeLists.txt
+++ b/bolt/lib/Core/CMakeLists.txt
@@ -25,6 +25,7 @@ add_llvm_library(LLVMBOLTCore
DynoStats.cpp
Exceptions.cpp
FunctionLayout.cpp
+ GDBIndex.cpp
HashUtilities.cpp
JumpTable.cpp
MCPlusBuilder.cpp
diff --git a/bolt/lib/Core/GDBIndex.cpp b/bolt/lib/Core/GDBIndex.cpp
new file mode 100644
index 00000000000000..fda19a2c8cb3ef
--- /dev/null
+++ b/bolt/lib/Core/GDBIndex.cpp
@@ -0,0 +1,182 @@
+//===- bolt/Rewrite/GDBIndex.cpp -------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "bolt/Core/GDBIndex.h"
+
+using namespace llvm::bolt;
+using namespace llvm::support::endian;
+
+void GDBIndex::addGDBTypeUnitEntry(const GDBIndexTUEntry &Entry) {
+ GDBIndexTUEntryVector.push_back(Entry);
+}
+
+void GDBIndex::updateGdbIndexSection(
+ CUOffsetMap &CUMap, uint32_t NumCUs,
+ std::unique_ptr<DebugARangesSectionWriter> &ARangesSectionWriter) {
+ if (!BC.getGdbIndexSection())
+ return;
+
+ // See https://sourceware.org/gdb/onlinedocs/gdb/Index-Section-Format.html
+ // for .gdb_index section format.
+
+ StringRef GdbIndexContents = BC.getGdbIndexSection()->getContents();
+
+ const char *Data = GdbIndexContents.data();
+
+ // Parse the header.
+ const uint32_t Version = read32le(Data);
+ if (Version != 7 && Version != 8) {
+ errs() << "BOLT-ERROR: can only process .gdb_index versions 7 and 8\n";
+ exit(1);
+ }
+
+ // Some .gdb_index generators use file offsets while others use section
+ // offsets. Hence we can only rely on offsets relative to each other,
+ // and ignore their absolute values.
+ const uint32_t CUListOffset = read32le(Data + 4);
+ const uint32_t CUTypesOffset = read32le(Data + 8);
+ const uint32_t AddressTableOffset = read32le(Data + 12);
+ const uint32_t SymbolTableOffset = read32le(Data + 16);
+ const uint32_t ConstantPoolOffset = read32le(Data + 20);
+ Data += 24;
+
+ // Map CUs offsets to indices and verify existing index table.
+ std::map<uint32_t, uint32_t> OffsetToIndexMap;
+ const uint32_t CUListSize = CUTypesOffset - CUListOffset;
+ const uint32_t TUListSize = AddressTableOffset - CUTypesOffset;
+ const unsigned NUmCUsEncoded = CUListSize / 16;
+ unsigned MaxDWARFVersion = BC.DwCtx->getMaxVersion();
+ unsigned NumDWARF5TUs =
+ getGDBIndexTUEntryVector().size() - BC.DwCtx->getNumTypeUnits();
+ bool SkipTypeUnits = false;
+ // For DWARF5 Types are in .debug_info.
+ // LLD doesn't generate Types CU List, and in CU list offset
+ // only includes CUs.
+ // GDB 11+ includes only CUs in CU list and generates Types
+ // list.
+ // GDB 9 includes CUs and TUs in CU list and generates TYpes
+ // list. The NumCUs is CUs + TUs, so need to modify the check.
+ // For split-dwarf
+ // GDB-11, DWARF5: TU units from dwo are not included.
+ // GDB-11, DWARF4: TU units from dwo are included.
+ if (MaxDWARFVersion >= 5)
+ SkipTypeUnits = !TUListSize ? true
+ : ((NUmCUsEncoded + NumDWARF5TUs) ==
+ BC.DwCtx->getNumCompileUnits());
+
+ if (!((CUListSize == NumCUs * 16) ||
+ (CUListSize == (NumCUs + NumDWARF5TUs) * 16))) {
+ errs() << "BOLT-ERROR: .gdb_index: CU count mismatch\n";
+ exit(1);
+ }
+ DenseSet<uint64_t> OriginalOffsets;
+ for (unsigned Index = 0, Units = BC.DwCtx->getNumCompileUnits();
+ Index < Units; ++Index) {
+ const DWARFUnit *CU = BC.DwCtx->getUnitAtIndex(Index);
+ if (SkipTypeUnits && CU->isTypeUnit())
+ continue;
+ const uint64_t Offset = read64le(Data);
+ Data += 16;
+ if (CU->getOffset() != Offset) {
+ errs() << "BOLT-ERROR: .gdb_index CU offset mismatch\n";
+ exit(1);
+ }
+
+ OriginalOffsets.insert(Offset);
+ OffsetToIndexMap[Offset] = Index;
+ }
+
+ // Ignore old address table.
+ const uint32_t OldAddressTableSize = SymbolTableOffset - AddressTableOffset;
+ // Move Data to the beginning of symbol table.
+ Data += SymbolTableOffset - CUTypesOffset;
+
+ // Calculate the size of the new address table.
+ uint32_t NewAddressTableSize = 0;
+ for (const auto &CURangesPair : ARangesSectionWriter->getCUAddressRanges()) {
+ const SmallVector<DebugAddressRange, 2> &Ranges = CURangesPair.second;
+ NewAddressTableSize += Ranges.size() * 20;
+ }
+
+ // Difference between old and new table (and section) sizes.
+ // Could be negative.
+ int32_t Delta = NewAddressTableSize - OldAddressTableSize;
+
+ size_t NewGdbIndexSize = GdbIndexContents.size() + Delta;
+
+ // Free'd by ExecutableFileMemoryManager.
+ auto *NewGdbIndexContents = new uint8_t[NewGdbIndexSize];
+ uint8_t *Buffer = NewGdbIndexContents;
+
+ write32le(Buffer, Version);
+ write32le(Buffer + 4, CUListOffset);
+ write32le(Buffer + 8, CUTypesOffset);
+ write32le(Buffer + 12, AddressTableOffset);
+ write32le(Buffer + 16, SymbolTableOffset + Delta);
+ write32le(Buffer + 20, ConstantPoolOffset + Delta);
+ Buffer += 24;
+
+ using MapEntry = std::pair<uint32_t, CUInfo>;
+ std::vector<MapEntry> CUVector(CUMap.begin(), CUMap.end());
+ // Need to sort since we write out all of TUs in .debug_info before CUs.
+ std::sort(CUVector.begin(), CUVector.end(),
+ [](const MapEntry &E1, const MapEntry &E2) -> bool {
+ return E1.second.Offset < E2.second.Offset;
+ });
+ // Writing out CU List <Offset, Size>
+ for (auto &CUInfo : CUVector) {
+ // Skipping TU for DWARF5 when they are not included in CU list.
+ if (!OriginalOffsets.count(CUInfo.first))
+ continue;
+ write64le(Buffer, CUInfo.second.Offset);
+ // Length encoded in CU doesn't contain first 4 bytes that encode length.
+ write64le(Buffer + 8, CUInfo.second.Length + 4);
+ Buffer += 16;
+ }
+
+ // Rewrite TU CU List, since abbrevs can be different.
+ // Entry example:
+ // 0: offset = 0x00000000, type_offset = 0x0000001e, type_signature =
+ // 0x418503b8111e9a7b Spec says " triplet, the first value is the CU offset,
+ // the second value is the type offset in the CU, and the third value is the
+ // type signature" Looking at what is being generated by gdb-add-index. The
+ // first entry is TU offset, second entry is offset from it, and third entry
+ // is the type signature.
+ if (TUListSize)
+ for (const GDBIndexTUEntry &Entry : getGDBIndexTUEntryVector()) {
+ write64le(Buffer, Entry.UnitOffset);
+ write64le(Buffer + 8, Entry.TypeDIERelativeOffset);
+ write64le(Buffer + 16, Entry.TypeHash);
+ Buffer += sizeof(GDBIndexTUEntry);
+ }
+
+ // Generate new address table.
+ for (const std::pair<const uint64_t, DebugAddressRangesVector> &CURangesPair :
+ ARangesSectionWriter->getCUAddressRanges()) {
+ const uint32_t CUIndex = OffsetToIndexMap[CURangesPair.first];
+ const DebugAddressRangesVector &Ranges = CURangesPair.second;
+ for (const DebugAddressRange &Range : Ranges) {
+ write64le(Buffer, Range.LowPC);
+ write64le(Buffer + 8, Range.HighPC);
+ write32le(Buffer + 16, CUIndex);
+ Buffer += 20;
+ }
+ }
+
+ const size_t TrailingSize =
+ GdbIndexContents.data() + GdbIndexContents.size() - Data;
+ assert(Buffer + TrailingSize == NewGdbIndexContents + NewGdbIndexSize &&
+ "size calculation error");
+
+ // Copy over the rest of the original data.
+ memcpy(Buffer, Data, TrailingSize);
+
+ // Register the new section.
+ BC.registerOrUpdateNoteSection(".gdb_index", NewGdbIndexContents,
+ NewGdbIndexSize);
+}
More information about the lldb-commits
mailing list