[llvm] [BOLT] Clean up jump table handling in non-reloc mode. NFCI (PR #119614)

Maksim Panchenko via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 11 13:01:03 PST 2024


https://github.com/maksfb updated https://github.com/llvm/llvm-project/pull/119614

>From b9acd3fac9f19560dd5f0497a58b3dbdf9cdb049 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Thu, 16 Sep 2021 10:16:44 -0700
Subject: [PATCH] [BOLT] Clean up jump table handling for non-reloc. NFCI

This change affects non-relocation mode only. Prior to having
CheckLargeFunctions pass, we could have emitted code for functions that
was discarded at the end due to size limitations. Since we didn't know
at the time of emission if the code would be discarded or not, we had to
emit jump tables in separate sections and handle them separately.
However, now we always run CheckLargeFunctions and make sure all emitted
code is used. Thus, we can get rid of the special jump table handling.
---
 bolt/lib/Core/BinaryEmitter.cpp      | 26 +++++--------------
 bolt/lib/Rewrite/RewriteInstance.cpp | 37 ++--------------------------
 2 files changed, 8 insertions(+), 55 deletions(-)

diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index f34a94c5779213..1744c1e5717224 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -730,30 +730,16 @@ void BinaryEmitter::emitJumpTables(const BinaryFunction &BF) {
       continue;
     if (opts::PrintJumpTables)
       JT.print(BC.outs());
-    if (opts::JumpTables == JTS_BASIC && BC.HasRelocations) {
+    if (opts::JumpTables == JTS_BASIC) {
       JT.updateOriginal();
     } else {
       MCSection *HotSection, *ColdSection;
-      if (opts::JumpTables == JTS_BASIC) {
-        // In non-relocation mode we have to emit jump tables in local sections.
-        // This way we only overwrite them when the corresponding function is
-        // overwritten.
-        std::string Name = ".local." + JT.Labels[0]->getName().str();
-        std::replace(Name.begin(), Name.end(), '/', '.');
-        BinarySection &Section =
-            BC.registerOrUpdateSection(Name, ELF::SHT_PROGBITS, ELF::SHF_ALLOC);
-        Section.setAnonymous(true);
-        JT.setOutputSection(Section);
-        HotSection = BC.getDataSection(Name);
-        ColdSection = HotSection;
+      if (BF.isSimple()) {
+        HotSection = ReadOnlySection;
+        ColdSection = ReadOnlyColdSection;
       } else {
-        if (BF.isSimple()) {
-          HotSection = ReadOnlySection;
-          ColdSection = ReadOnlyColdSection;
-        } else {
-          HotSection = BF.hasProfile() ? ReadOnlySection : ReadOnlyColdSection;
-          ColdSection = HotSection;
-        }
+        HotSection = BF.hasProfile() ? ReadOnlySection : ReadOnlyColdSection;
+        ColdSection = HotSection;
       }
       emitJumpTable(JT, HotSection, ColdSection);
     }
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 76e1f0156f828d..84fd8fd591eab4 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -78,7 +78,6 @@ namespace opts {
 extern cl::list<std::string> HotTextMoveSections;
 extern cl::opt<bool> Hugify;
 extern cl::opt<bool> Instrument;
-extern cl::opt<JumpTableSupportLevel> JumpTables;
 extern cl::opt<bool> KeepNops;
 extern cl::opt<bool> Lite;
 extern cl::list<std::string> ReorderData;
@@ -3841,20 +3840,6 @@ void RewriteInstance::mapCodeSections(BOLTLinker::SectionMapper MapSection) {
     assert(Function.getImageSize() <= Function.getMaxSize() &&
            "Unexpected large function");
 
-    // Map jump tables if updating in-place.
-    if (opts::JumpTables == JTS_BASIC) {
-      for (auto &JTI : Function.JumpTables) {
-        JumpTable *JT = JTI.second;
-        BinarySection &Section = JT->getOutputSection();
-        Section.setOutputAddress(JT->getAddress());
-        Section.setOutputFileOffset(getFileOffsetForAddress(JT->getAddress()));
-        LLVM_DEBUG(dbgs() << "BOLT-DEBUG: mapping JT " << Section.getName()
-                          << " to 0x" << Twine::utohexstr(JT->getAddress())
-                          << '\n');
-        MapSection(Section, JT->getAddress());
-      }
-    }
-
     if (!Function.isSplit())
       continue;
 
@@ -5637,26 +5622,8 @@ void RewriteInstance::rewriteFile() {
     if (Function->getImageAddress() == 0 || Function->getImageSize() == 0)
       continue;
 
-    if (Function->getImageSize() > Function->getMaxSize()) {
-      assert(!BC->isX86() && "Unexpected large function.");
-      if (opts::Verbosity >= 1)
-        BC->errs() << "BOLT-WARNING: new function size (0x"
-                   << Twine::utohexstr(Function->getImageSize())
-                   << ") is larger than maximum allowed size (0x"
-                   << Twine::utohexstr(Function->getMaxSize())
-                   << ") for function " << *Function << '\n';
-
-      // Remove jump table sections that this function owns in non-reloc mode
-      // because we don't want to write them anymore.
-      if (!BC->HasRelocations && opts::JumpTables == JTS_BASIC) {
-        for (auto &JTI : Function->JumpTables) {
-          JumpTable *JT = JTI.second;
-          BinarySection &Section = JT->getOutputSection();
-          BC->deregisterSection(Section);
-        }
-      }
-      continue;
-    }
+    assert(Function->getImageSize() <= Function->getMaxSize() &&
+           "Unexpected large function");
 
     const auto HasAddress = [](const FunctionFragment &FF) {
       return FF.empty() ||



More information about the llvm-commits mailing list