[llvm] [CodeGen] Hacks solution to X86 CodeGen extern symbol duplication issue (PR #106737)
weiwei chen via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 30 08:17:39 PDT 2024
https://github.com/weiweichen updated https://github.com/llvm/llvm-project/pull/106737
>From 3214f1aa08d2a4cf254993f672101c97e3c9efdc Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 30 Aug 2024 04:25:59 +0000
Subject: [PATCH 1/4] Hacky fix.
---
.../llvm/ExecutionEngine/JITLink/JITLink.h | 3 +++
llvm/lib/Target/X86/X86MCInstLower.cpp | 15 ++++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 8fe53760a852d7..8d4daa8a77b736 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -1212,7 +1212,10 @@ class LinkGraph {
/// of 0.
Symbol &addExternalSymbol(StringRef Name, orc::ExecutorAddrDiff Size,
bool IsWeaklyReferenced) {
+ if(ExternalSymbols.contains(Name))
+ llvm::dbgs() << "Duplicate external symbol: " << Name << "\n";
assert(!ExternalSymbols.contains(Name) && "Duplicate external symbol");
+
auto &Sym = Symbol::constructExternal(
Allocator, createAddressable(orc::ExecutorAddr(), false), Name, Size,
Linkage::Strong, IsWeaklyReferenced);
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index 77ddd2366e629e..d01085fd5a532c 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -80,7 +80,7 @@ class X86MCInstLower {
const MachineOperand &MO) const;
void Lower(const MachineInstr *MI, MCInst &OutMI) const;
- MCSymbol *GetSymbolFromOperand(const MachineOperand &MO) const;
+ MCSymbol *GetSymbolFromOperand(const MachineOperand &MO, MCContext* ExternalContext=nullptr) const;
MCOperand LowerSymbolOperand(const MachineOperand &MO, MCSymbol *Sym) const;
private:
@@ -152,7 +152,7 @@ MachineModuleInfoMachO &X86MCInstLower::getMachOMMI() const {
/// GetSymbolFromOperand - Lower an MO_GlobalAddress or MO_ExternalSymbol
/// operand to an MCSymbol.
-MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
+MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO, MCContext* ExternalContext) const {
const Triple &TT = TM.getTargetTriple();
if (MO.isGlobal() && TT.isOSBinFormatELF())
return AsmPrinter.getSymbolPreferLocal(*MO.getGlobal());
@@ -193,8 +193,13 @@ MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
}
Name += Suffix;
- if (!Sym)
- Sym = Ctx.getOrCreateSymbol(Name);
+ if (!Sym) {
+ if (ExternalContext) {
+ Sym = ExternalContext->getOrCreateSymbol(Name);
+ } else {
+ Sym = Ctx.getOrCreateSymbol(Name);
+ }
+ }
// If the target flags on the operand changes the name of the symbol, do that
// before we return the symbol.
@@ -351,7 +356,7 @@ MCOperand X86MCInstLower::LowerMachineOperand(const MachineInstr *MI,
case MachineOperand::MO_MachineBasicBlock:
case MachineOperand::MO_GlobalAddress:
case MachineOperand::MO_ExternalSymbol:
- return LowerSymbolOperand(MO, GetSymbolFromOperand(MO));
+ return LowerSymbolOperand(MO, GetSymbolFromOperand(MO, &AsmPrinter.OutContext));
case MachineOperand::MO_MCSymbol:
return LowerSymbolOperand(MO, MO.getMCSymbol());
case MachineOperand::MO_JumpTableIndex:
>From 859aacabfdf1ecef25656c88287e77d985bfcce2 Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 30 Aug 2024 15:03:45 +0000
Subject: [PATCH 2/4] Formaaaaaaaaaattttttttt.
---
.../llvm/ExecutionEngine/JITLink/JITLink.h | 32 ++++++++-----------
llvm/lib/Target/X86/X86MCInstLower.cpp | 10 ++++--
2 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 8d4daa8a77b736..e2dbbc318b7480 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -15,8 +15,8 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ExecutionEngine/JITLink/JITLinkMemoryManager.h"
#include "llvm/ExecutionEngine/JITSymbol.h"
@@ -389,11 +389,7 @@ const char *getLinkageName(Linkage L);
/// Default -- Visible in the public interface of the linkage unit.
/// Hidden -- Visible within the linkage unit, but not exported from it.
/// Local -- Visible only within the LinkGraph.
-enum class Scope : uint8_t {
- Default,
- Hidden,
- Local
-};
+enum class Scope : uint8_t { Default, Hidden, Local };
/// For debugging output.
const char *getScopeName(Scope S);
@@ -853,8 +849,7 @@ class LinkGraph {
using AbsoluteSymbolSet = DenseSet<Symbol *>;
using BlockSet = DenseSet<Block *>;
- template <typename... ArgTs>
- Addressable &createAddressable(ArgTs &&... Args) {
+ template <typename... ArgTs> Addressable &createAddressable(ArgTs &&...Args) {
Addressable *A =
reinterpret_cast<Addressable *>(Allocator.Allocate<Addressable>());
new (A) Addressable(std::forward<ArgTs>(Args)...);
@@ -866,7 +861,7 @@ class LinkGraph {
Allocator.Deallocate(&A);
}
- template <typename... ArgTs> Block &createBlock(ArgTs &&... Args) {
+ template <typename... ArgTs> Block &createBlock(ArgTs &&...Args) {
Block *B = reinterpret_cast<Block *>(Allocator.Allocate<Block>());
new (B) Block(std::forward<ArgTs>(Args)...);
B->getSection().addBlock(*B);
@@ -1212,8 +1207,8 @@ class LinkGraph {
/// of 0.
Symbol &addExternalSymbol(StringRef Name, orc::ExecutorAddrDiff Size,
bool IsWeaklyReferenced) {
- if(ExternalSymbols.contains(Name))
- llvm::dbgs() << "Duplicate external symbol: " << Name << "\n";
+ if (ExternalSymbols.contains(Name))
+ llvm::dbgs() << "Duplicate external symbol: " << Name << "\n";
assert(!ExternalSymbols.contains(Name) && "Duplicate external symbol");
auto &Sym = Symbol::constructExternal(
@@ -1228,10 +1223,10 @@ class LinkGraph {
orc::ExecutorAddrDiff Size, Linkage L, Scope S,
bool IsLive) {
assert((S == Scope::Local || llvm::count_if(AbsoluteSymbols,
- [&](const Symbol *Sym) {
- return Sym->getName() == Name;
- }) == 0) &&
- "Duplicate absolute symbol");
+ [&](const Symbol *Sym) {
+ return Sym->getName() == Name;
+ }) == 0) &&
+ "Duplicate absolute symbol");
auto &Sym = Symbol::constructAbsolute(Allocator, createAddressable(Address),
Name, Size, L, S, IsLive);
AbsoluteSymbols.insert(&Sym);
@@ -1508,10 +1503,9 @@ class LinkGraph {
/// Remove a block. The block reference is defunct after calling this
/// function and should no longer be used.
void removeBlock(Block &B) {
- assert(llvm::none_of(B.getSection().symbols(),
- [&](const Symbol *Sym) {
- return &Sym->getBlock() == &B;
- }) &&
+ assert(llvm::none_of(
+ B.getSection().symbols(),
+ [&](const Symbol *Sym) { return &Sym->getBlock() == &B; }) &&
"Block still has symbols attached");
B.getSection().removeBlock(B);
destroyBlock(B);
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index d01085fd5a532c..6c38130912bbda 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -80,7 +80,8 @@ class X86MCInstLower {
const MachineOperand &MO) const;
void Lower(const MachineInstr *MI, MCInst &OutMI) const;
- MCSymbol *GetSymbolFromOperand(const MachineOperand &MO, MCContext* ExternalContext=nullptr) const;
+ MCSymbol *GetSymbolFromOperand(const MachineOperand &MO,
+ MCContext* ExternalContext = nullptr) const;
MCOperand LowerSymbolOperand(const MachineOperand &MO, MCSymbol *Sym) const;
private:
@@ -152,7 +153,9 @@ MachineModuleInfoMachO &X86MCInstLower::getMachOMMI() const {
/// GetSymbolFromOperand - Lower an MO_GlobalAddress or MO_ExternalSymbol
/// operand to an MCSymbol.
-MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO, MCContext* ExternalContext) const {
+MCSymbol *
+X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO,
+ MCContext* ExternalContext) const {
const Triple &TT = TM.getTargetTriple();
if (MO.isGlobal() && TT.isOSBinFormatELF())
return AsmPrinter.getSymbolPreferLocal(*MO.getGlobal());
@@ -356,7 +359,8 @@ MCOperand X86MCInstLower::LowerMachineOperand(const MachineInstr *MI,
case MachineOperand::MO_MachineBasicBlock:
case MachineOperand::MO_GlobalAddress:
case MachineOperand::MO_ExternalSymbol:
- return LowerSymbolOperand(MO, GetSymbolFromOperand(MO, &AsmPrinter.OutContext));
+ return LowerSymbolOperand(MO,
+ GetSymbolFromOperand(MO, &AsmPrinter.OutContext));
case MachineOperand::MO_MCSymbol:
return LowerSymbolOperand(MO, MO.getMCSymbol());
case MachineOperand::MO_JumpTableIndex:
>From 51526435037476eb292378903745e7964123147e Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 30 Aug 2024 15:14:35 +0000
Subject: [PATCH 3/4] Revert change in JITLink.
---
.../llvm/ExecutionEngine/JITLink/JITLink.h | 31 ++++++++++---------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index e2dbbc318b7480..8fe53760a852d7 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -15,8 +15,8 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ExecutionEngine/JITLink/JITLinkMemoryManager.h"
#include "llvm/ExecutionEngine/JITSymbol.h"
@@ -389,7 +389,11 @@ const char *getLinkageName(Linkage L);
/// Default -- Visible in the public interface of the linkage unit.
/// Hidden -- Visible within the linkage unit, but not exported from it.
/// Local -- Visible only within the LinkGraph.
-enum class Scope : uint8_t { Default, Hidden, Local };
+enum class Scope : uint8_t {
+ Default,
+ Hidden,
+ Local
+};
/// For debugging output.
const char *getScopeName(Scope S);
@@ -849,7 +853,8 @@ class LinkGraph {
using AbsoluteSymbolSet = DenseSet<Symbol *>;
using BlockSet = DenseSet<Block *>;
- template <typename... ArgTs> Addressable &createAddressable(ArgTs &&...Args) {
+ template <typename... ArgTs>
+ Addressable &createAddressable(ArgTs &&... Args) {
Addressable *A =
reinterpret_cast<Addressable *>(Allocator.Allocate<Addressable>());
new (A) Addressable(std::forward<ArgTs>(Args)...);
@@ -861,7 +866,7 @@ class LinkGraph {
Allocator.Deallocate(&A);
}
- template <typename... ArgTs> Block &createBlock(ArgTs &&...Args) {
+ template <typename... ArgTs> Block &createBlock(ArgTs &&... Args) {
Block *B = reinterpret_cast<Block *>(Allocator.Allocate<Block>());
new (B) Block(std::forward<ArgTs>(Args)...);
B->getSection().addBlock(*B);
@@ -1207,10 +1212,7 @@ class LinkGraph {
/// of 0.
Symbol &addExternalSymbol(StringRef Name, orc::ExecutorAddrDiff Size,
bool IsWeaklyReferenced) {
- if (ExternalSymbols.contains(Name))
- llvm::dbgs() << "Duplicate external symbol: " << Name << "\n";
assert(!ExternalSymbols.contains(Name) && "Duplicate external symbol");
-
auto &Sym = Symbol::constructExternal(
Allocator, createAddressable(orc::ExecutorAddr(), false), Name, Size,
Linkage::Strong, IsWeaklyReferenced);
@@ -1223,10 +1225,10 @@ class LinkGraph {
orc::ExecutorAddrDiff Size, Linkage L, Scope S,
bool IsLive) {
assert((S == Scope::Local || llvm::count_if(AbsoluteSymbols,
- [&](const Symbol *Sym) {
- return Sym->getName() == Name;
- }) == 0) &&
- "Duplicate absolute symbol");
+ [&](const Symbol *Sym) {
+ return Sym->getName() == Name;
+ }) == 0) &&
+ "Duplicate absolute symbol");
auto &Sym = Symbol::constructAbsolute(Allocator, createAddressable(Address),
Name, Size, L, S, IsLive);
AbsoluteSymbols.insert(&Sym);
@@ -1503,9 +1505,10 @@ class LinkGraph {
/// Remove a block. The block reference is defunct after calling this
/// function and should no longer be used.
void removeBlock(Block &B) {
- assert(llvm::none_of(
- B.getSection().symbols(),
- [&](const Symbol *Sym) { return &Sym->getBlock() == &B; }) &&
+ assert(llvm::none_of(B.getSection().symbols(),
+ [&](const Symbol *Sym) {
+ return &Sym->getBlock() == &B;
+ }) &&
"Block still has symbols attached");
B.getSection().removeBlock(B);
destroyBlock(B);
>From 3b661b1948dc2c4e2b354661acc41427f15a48ff Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 30 Aug 2024 15:17:24 +0000
Subject: [PATCH 4/4] Formaaaaaaatttttt againnnnnnnnnnn.
---
llvm/lib/Target/X86/X86MCInstLower.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index 6c38130912bbda..2f1312c1b7072c 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -81,7 +81,7 @@ class X86MCInstLower {
void Lower(const MachineInstr *MI, MCInst &OutMI) const;
MCSymbol *GetSymbolFromOperand(const MachineOperand &MO,
- MCContext* ExternalContext = nullptr) const;
+ MCContext *ExternalContext = nullptr) const;
MCOperand LowerSymbolOperand(const MachineOperand &MO, MCSymbol *Sym) const;
private:
@@ -155,7 +155,7 @@ MachineModuleInfoMachO &X86MCInstLower::getMachOMMI() const {
/// operand to an MCSymbol.
MCSymbol *
X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO,
- MCContext* ExternalContext) const {
+ MCContext *ExternalContext) const {
const Triple &TT = TM.getTargetTriple();
if (MO.isGlobal() && TT.isOSBinFormatELF())
return AsmPrinter.getSymbolPreferLocal(*MO.getGlobal());
More information about the llvm-commits
mailing list