[llvm] [BOLT][NFC] Simplify getOrCreate/analyze/populate/emitJumpTable (PR #132108)
Amir Ayupov via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 10 13:02:21 PDT 2025
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/132108
>From 7bf6639a6e09e5ae2c78291c4da4faf160904dfa Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 19 Mar 2025 14:57:49 -0700
Subject: [PATCH 1/4] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
---
bolt/lib/Core/BinaryContext.cpp | 78 ++++++++++++++-------------------
1 file changed, 32 insertions(+), 46 deletions(-)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 8fa1e367c685e..c4902653a9b01 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -645,24 +645,19 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
// Function or one of its fragments.
const BinaryFunction *TargetBF = getBinaryFunctionContainingAddress(Value);
- const bool DoesBelongToFunction =
- BF.containsAddress(Value) ||
- (TargetBF && areRelatedFragments(TargetBF, &BF));
- if (!DoesBelongToFunction) {
+ if (!TargetBF || !areRelatedFragments(TargetBF, &BF)) {
LLVM_DEBUG({
- if (!BF.containsAddress(Value)) {
- dbgs() << "FAIL: function doesn't contain this address\n";
- if (TargetBF) {
- dbgs() << " ! function containing this address: "
- << TargetBF->getPrintName() << '\n';
- if (TargetBF->isFragment()) {
- dbgs() << " ! is a fragment";
- for (BinaryFunction *Parent : TargetBF->ParentFragments)
- dbgs() << ", parent: " << Parent->getPrintName();
- dbgs() << '\n';
- }
- }
- }
+ dbgs() << "FAIL: function doesn't contain this address\n";
+ if (!TargetBF)
+ break;
+ dbgs() << " ! function containing this address: " << *TargetBF << '\n';
+ if (!TargetBF->isFragment())
+ break;
+ dbgs() << " ! is a fragment with parents: ";
+ ListSeparator LS;
+ for (BinaryFunction *Parent : TargetBF->ParentFragments)
+ dbgs() << LS << *Parent;
+ dbgs() << '\n';
});
break;
}
@@ -702,10 +697,8 @@ void BinaryContext::populateJumpTables() {
++JTI) {
JumpTable *JT = JTI->second;
- bool NonSimpleParent = false;
- for (BinaryFunction *BF : JT->Parents)
- NonSimpleParent |= !BF->isSimple();
- if (NonSimpleParent)
+ auto isSimple = std::bind(&BinaryFunction::isSimple, std::placeholders::_1);
+ if (!llvm::all_of(JT->Parents, isSimple))
continue;
uint64_t NextJTAddress = 0;
@@ -839,33 +832,26 @@ BinaryContext::getOrCreateJumpTable(BinaryFunction &Function, uint64_t Address,
assert(JT->Type == Type && "jump table types have to match");
assert(Address == JT->getAddress() && "unexpected non-empty jump table");
- // Prevent associating a jump table to a specific fragment twice.
- if (!llvm::is_contained(JT->Parents, &Function)) {
- assert(llvm::all_of(JT->Parents,
- [&](const BinaryFunction *BF) {
- return areRelatedFragments(&Function, BF);
- }) &&
- "cannot re-use jump table of a different function");
- // Duplicate the entry for the parent function for easy access
- JT->Parents.push_back(&Function);
- if (opts::Verbosity > 2) {
- this->outs() << "BOLT-INFO: Multiple fragments access same jump table: "
- << JT->Parents[0]->getPrintName() << "; "
- << Function.getPrintName() << "\n";
- JT->print(this->outs());
- }
- Function.JumpTables.emplace(Address, JT);
- for (BinaryFunction *Parent : JT->Parents)
- Parent->setHasIndirectTargetToSplitFragment(true);
- }
+ if (llvm::is_contained(JT->Parents, &Function))
+ return JT->getFirstLabel();
- bool IsJumpTableParent = false;
- (void)IsJumpTableParent;
- for (BinaryFunction *Frag : JT->Parents)
- if (Frag == &Function)
- IsJumpTableParent = true;
- assert(IsJumpTableParent &&
+ // Prevent associating a jump table to a specific fragment twice.
+ auto isSibling = std::bind(&BinaryContext::areRelatedFragments, this,
+ &Function, std::placeholders::_1);
+ assert(llvm::all_of(JT->Parents, isSibling) &&
"cannot re-use jump table of a different function");
+ if (opts::Verbosity > 2) {
+ this->outs() << "BOLT-INFO: Multiple fragments access same jump table: "
+ << JT->Parents[0]->getPrintName() << "; "
+ << Function.getPrintName() << "\n";
+ JT->print(this->outs());
+ }
+ if (JT->Parents.size() == 1)
+ JT->Parents.front()->setHasIndirectTargetToSplitFragment(true);
+ Function.setHasIndirectTargetToSplitFragment(true);
+ // Duplicate the entry for the parent function for easy access
+ JT->Parents.push_back(&Function);
+ Function.JumpTables.emplace(Address, JT);
return JT->getFirstLabel();
}
>From 2bd0d6dc27afae51ba50b2e778de420712ccd984 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Thu, 10 Apr 2025 10:58:08 -0700
Subject: [PATCH 2/4] s/LastLabel/JTLabel in emitJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryEmitter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 5c90e00aa0749..49f41bc9dafab 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -852,7 +852,7 @@ void BinaryEmitter::emitJumpTable(const JumpTable &JT, MCSection *HotSection,
Streamer.emitSymbolValue(Entry, JT.OutputEntrySize);
} else { // JTT_PIC
const MCSymbolRefExpr *JTExpr =
- MCSymbolRefExpr::create(LastLabel, Streamer.getContext());
+ MCSymbolRefExpr::create(JTLabel, Streamer.getContext());
const MCSymbolRefExpr *E =
MCSymbolRefExpr::create(Entry, Streamer.getContext());
const MCBinaryExpr *Value =
>From e7927a98a6783f8e00439707923016e9fd6aeae1 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Thu, 10 Apr 2025 11:06:08 -0700
Subject: [PATCH 3/4] update split-func-jump-table-fragment-bidirection.s
Created using spr 1.3.4
---
bolt/test/X86/split-func-jump-table-fragment-bidirection.s | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/test/X86/split-func-jump-table-fragment-bidirection.s b/bolt/test/X86/split-func-jump-table-fragment-bidirection.s
index 52c816ccd9005..3d3247fc58f25 100644
--- a/bolt/test/X86/split-func-jump-table-fragment-bidirection.s
+++ b/bolt/test/X86/split-func-jump-table-fragment-bidirection.s
@@ -10,7 +10,7 @@
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
# RUN: llvm-bolt -print-cfg -v=3 %t.exe -o %t.out 2>&1 | FileCheck %s
-# CHECK: BOLT-INFO: Multiple fragments access same jump table: main; main.cold.1
+# CHECK: BOLT-INFO: multiple fragments access the same jump table: main; main.cold.1
# CHECK: PIC Jump table JUMP_TABLE1 for function main, main.cold.1 at {{.*}} with a total count of 0:
.text
>From 919f4524609794f77c5b99abaca55aa3d2207921 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Thu, 10 Apr 2025 13:02:09 -0700
Subject: [PATCH 4/4] const
Created using spr 1.3.4
---
bolt/lib/Core/BinaryEmitter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 49f41bc9dafab..d3ed9e8088bfa 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -823,7 +823,7 @@ void BinaryEmitter::emitJumpTable(const JumpTable &JT, MCSection *HotSection,
<< (Offset ? ") as part of larger jump table\n" : ")\n");
});
if (!LabelCounts.empty()) {
- uint64_t JTCount = LabelCounts[JTLabel];
+ const uint64_t JTCount = LabelCounts[JTLabel];
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: jump table count: " << JTCount << '\n');
Streamer.switchSection(JTCount ? HotSection : ColdSection);
Streamer.emitValueToAlignment(Align(JT.EntrySize));
More information about the llvm-commits
mailing list