[llvm] [BOLT] Emit synthetic FILE symbol for local cold fragments of global symbols (PR #89794)
Amir Ayupov via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 24 18:09:51 PDT 2024
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/89794
>From 6d621e9d0b51fe21d8c1fc2c953242d788452579 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Tue, 23 Apr 2024 09:48:12 -0700
Subject: [PATCH 1/5] [BOLT] Emit bolt-synthetic FILE symbol
Follow-up to https://github.com/llvm/llvm-project/pull/89648.
Emit a FILE symbol to distinguish BOLT-created local symbols for split
functions, to prevent polluting the namespace of the last file symbol.
Test Plan: Updated cdsplit-symbol-names.s
---
bolt/lib/Rewrite/RewriteInstance.cpp | 11 +++++++++++
bolt/test/X86/cdsplit-symbol-names.s | 1 +
2 files changed, 12 insertions(+)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 4e0096cf988aed..edd2dacfb70e4a 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -4493,6 +4493,17 @@ void RewriteInstance::updateELFSymbolTable(
// Symbols for the new symbol table.
std::vector<ELFSymTy> Symbols;
+ // Prepend synthetic FILE symbol to prevent local cold fragments from
+ // colliding with existing symbols with the same name.
+ ELFSymTy FileSymbol;
+ FileSymbol.st_shndx =
+ BC->getUniqueSectionByName(BC->getColdCodeSectionName())->getIndex();
+ FileSymbol.st_name = AddToStrTab("bolt-synthetic");
+ FileSymbol.st_value = 0;
+ FileSymbol.st_size = 0;
+ FileSymbol.setBindingAndType(ELF::STB_LOCAL, ELF::STT_FILE);
+ Symbols.emplace_back(FileSymbol);
+
auto getNewSectionIndex = [&](uint32_t OldIndex) {
// For dynamic symbol table, the section index could be wrong on the input,
// and its value is ignored by the runtime if it's different from
diff --git a/bolt/test/X86/cdsplit-symbol-names.s b/bolt/test/X86/cdsplit-symbol-names.s
index e2259276e2554c..e6e686a381e14a 100644
--- a/bolt/test/X86/cdsplit-symbol-names.s
+++ b/bolt/test/X86/cdsplit-symbol-names.s
@@ -10,6 +10,7 @@
# RUN: --call-scale=2 --data=%t.fdata --reorder-blocks=ext-tsp
# RUN: llvm-objdump --syms %t.bolt | FileCheck %s --check-prefix=CHECK-SYMS-WARM
+# CHECK-SYMS-WARM: [[#%X,VALUE:0]] l df *ABS* [[#%X,SIZE:0]] bolt-synthetic
# CHECK-SYMS-WARM: .text.warm
# CHECK-SYMS-WARM-SAME: chain.warm
# CHECK-SYMS-WARM: .text.cold
>From 469f02aab12fb3152502ea8bdde06a637ae2ca5a Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Tue, 23 Apr 2024 10:34:00 -0700
Subject: [PATCH 2/5] fixup! [BOLT] Emit bolt-synthetic FILE symbol
---
bolt/lib/Rewrite/RewriteInstance.cpp | 25 +++++++++++++++----------
bolt/test/X86/cdsplit-symbol-names.s | 2 +-
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index edd2dacfb70e4a..833150b0d51768 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -4493,16 +4493,7 @@ void RewriteInstance::updateELFSymbolTable(
// Symbols for the new symbol table.
std::vector<ELFSymTy> Symbols;
- // Prepend synthetic FILE symbol to prevent local cold fragments from
- // colliding with existing symbols with the same name.
- ELFSymTy FileSymbol;
- FileSymbol.st_shndx =
- BC->getUniqueSectionByName(BC->getColdCodeSectionName())->getIndex();
- FileSymbol.st_name = AddToStrTab("bolt-synthetic");
- FileSymbol.st_value = 0;
- FileSymbol.st_size = 0;
- FileSymbol.setBindingAndType(ELF::STB_LOCAL, ELF::STT_FILE);
- Symbols.emplace_back(FileSymbol);
+ bool EmittedColdFileSymbol{false};
auto getNewSectionIndex = [&](uint32_t OldIndex) {
// For dynamic symbol table, the section index could be wrong on the input,
@@ -4562,6 +4553,20 @@ void RewriteInstance::updateELFSymbolTable(
Symbols.emplace_back(ICFSymbol);
}
if (Function.isSplit()) {
+ // Prepend synthetic FILE symbol to prevent local cold fragments from
+ // colliding with existing symbols with the same name.
+ if (!EmittedColdFileSymbol &&
+ FunctionSymbol.getBinding() == ELF::STB_GLOBAL) {
+ ELFSymTy FileSymbol;
+ FileSymbol.st_shndx = ELF::SHN_ABS;
+ FileSymbol.st_name = AddToStrTab("bolt_cold.o");
+ FileSymbol.st_value = 0;
+ FileSymbol.st_size = 0;
+ FileSymbol.st_other = 0;
+ FileSymbol.setBindingAndType(ELF::STB_LOCAL, ELF::STT_FILE);
+ Symbols.emplace_back(FileSymbol);
+ EmittedColdFileSymbol = true;
+ }
for (const FunctionFragment &FF :
Function.getLayout().getSplitFragments()) {
if (FF.getAddress()) {
diff --git a/bolt/test/X86/cdsplit-symbol-names.s b/bolt/test/X86/cdsplit-symbol-names.s
index e6e686a381e14a..8d27308afd0e21 100644
--- a/bolt/test/X86/cdsplit-symbol-names.s
+++ b/bolt/test/X86/cdsplit-symbol-names.s
@@ -10,7 +10,7 @@
# RUN: --call-scale=2 --data=%t.fdata --reorder-blocks=ext-tsp
# RUN: llvm-objdump --syms %t.bolt | FileCheck %s --check-prefix=CHECK-SYMS-WARM
-# CHECK-SYMS-WARM: [[#%X,VALUE:0]] l df *ABS* [[#%X,SIZE:0]] bolt-synthetic
+# CHECK-SYMS-WARM: 0000000000000000 l df *ABS* 0000000000000000 bolt_cold.o
# CHECK-SYMS-WARM: .text.warm
# CHECK-SYMS-WARM-SAME: chain.warm
# CHECK-SYMS-WARM: .text.cold
>From d2e6ac23715e6ec43229be4b39addc95eb82e13f Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 24 Apr 2024 17:05:39 -0700
Subject: [PATCH 3/5] fixup! fixup! [BOLT] Emit bolt-synthetic FILE symbol
---
bolt/lib/Rewrite/RewriteInstance.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 833150b0d51768..1c1138159735be 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -4493,7 +4493,7 @@ void RewriteInstance::updateELFSymbolTable(
// Symbols for the new symbol table.
std::vector<ELFSymTy> Symbols;
- bool EmittedColdFileSymbol{false};
+ bool EmittedColdFileSymbol = false;
auto getNewSectionIndex = [&](uint32_t OldIndex) {
// For dynamic symbol table, the section index could be wrong on the input,
@@ -4559,7 +4559,7 @@ void RewriteInstance::updateELFSymbolTable(
FunctionSymbol.getBinding() == ELF::STB_GLOBAL) {
ELFSymTy FileSymbol;
FileSymbol.st_shndx = ELF::SHN_ABS;
- FileSymbol.st_name = AddToStrTab("bolt_cold.o");
+ FileSymbol.st_name = AddToStrTab("llvm-bolt-pseudo.o");
FileSymbol.st_value = 0;
FileSymbol.st_size = 0;
FileSymbol.st_other = 0;
>From 9461d38b7df591d32cf81019e0d4f8221403a355 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 24 Apr 2024 17:14:26 -0700
Subject: [PATCH 4/5] fixup! fixup! fixup! [BOLT] Emit bolt-synthetic FILE
symbol
---
bolt/test/X86/cdsplit-symbol-names.s | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/test/X86/cdsplit-symbol-names.s b/bolt/test/X86/cdsplit-symbol-names.s
index 8d27308afd0e21..04fecf424ffcae 100644
--- a/bolt/test/X86/cdsplit-symbol-names.s
+++ b/bolt/test/X86/cdsplit-symbol-names.s
@@ -10,7 +10,7 @@
# RUN: --call-scale=2 --data=%t.fdata --reorder-blocks=ext-tsp
# RUN: llvm-objdump --syms %t.bolt | FileCheck %s --check-prefix=CHECK-SYMS-WARM
-# CHECK-SYMS-WARM: 0000000000000000 l df *ABS* 0000000000000000 bolt_cold.o
+# CHECK-SYMS-WARM: 0000000000000000 l df *ABS* 0000000000000000 llvm-bolt-pseudo.o
# CHECK-SYMS-WARM: .text.warm
# CHECK-SYMS-WARM-SAME: chain.warm
# CHECK-SYMS-WARM: .text.cold
>From 25cd1f263ab6e4ecb9ca484eaadf948902fe1c48 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 24 Apr 2024 18:09:35 -0700
Subject: [PATCH 5/5] Define getBOLTFileSymbolName
---
bolt/include/bolt/Rewrite/RewriteInstance.h | 3 +++
bolt/lib/Rewrite/RewriteInstance.cpp | 2 +-
bolt/test/X86/cdsplit-symbol-names.s | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h
index af832b4c7c84cf..2561468a0f99af 100644
--- a/bolt/include/bolt/Rewrite/RewriteInstance.h
+++ b/bolt/include/bolt/Rewrite/RewriteInstance.h
@@ -426,6 +426,9 @@ class RewriteInstance {
static StringRef getEHFrameSectionName() { return ".eh_frame"; }
static StringRef getRelaDynSectionName() { return ".rela.dyn"; }
+ /// FILE symbol name used for local fragments of global functions.
+ static StringRef getBOLTFileSymbolName() { return "bolt-pseudo.o"; }
+
/// An instance of the input binary we are processing, externally owned.
llvm::object::ELFObjectFileBase *InputFile;
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 1c1138159735be..0cccebc348a3ab 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -4559,7 +4559,7 @@ void RewriteInstance::updateELFSymbolTable(
FunctionSymbol.getBinding() == ELF::STB_GLOBAL) {
ELFSymTy FileSymbol;
FileSymbol.st_shndx = ELF::SHN_ABS;
- FileSymbol.st_name = AddToStrTab("llvm-bolt-pseudo.o");
+ FileSymbol.st_name = AddToStrTab(getBOLTFileSymbolName());
FileSymbol.st_value = 0;
FileSymbol.st_size = 0;
FileSymbol.st_other = 0;
diff --git a/bolt/test/X86/cdsplit-symbol-names.s b/bolt/test/X86/cdsplit-symbol-names.s
index 04fecf424ffcae..e53863e22246d6 100644
--- a/bolt/test/X86/cdsplit-symbol-names.s
+++ b/bolt/test/X86/cdsplit-symbol-names.s
@@ -10,7 +10,7 @@
# RUN: --call-scale=2 --data=%t.fdata --reorder-blocks=ext-tsp
# RUN: llvm-objdump --syms %t.bolt | FileCheck %s --check-prefix=CHECK-SYMS-WARM
-# CHECK-SYMS-WARM: 0000000000000000 l df *ABS* 0000000000000000 llvm-bolt-pseudo.o
+# CHECK-SYMS-WARM: 0000000000000000 l df *ABS* 0000000000000000 bolt-pseudo.o
# CHECK-SYMS-WARM: .text.warm
# CHECK-SYMS-WARM-SAME: chain.warm
# CHECK-SYMS-WARM: .text.cold
More information about the llvm-commits
mailing list