[llvm] [JITLink][i386] Avoid 'i386' name clashing with built-in macro (PR #137063)
Dimitry Andric via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 23 14:23:50 PDT 2025
https://github.com/DimitryAndric created https://github.com/llvm/llvm-project/pull/137063
When compiling llvm on an actual i386 platform, both clang and gcc define a built-in macro `i386` to the value 1. This clashes with the `llvm::jitlink::i386` namespace:
```
In file included from llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp:18:
llvm/include/llvm/ExecutionEngine/JITLink/i386.h:19:24: error: expected '{'
19 | namespace llvm::jitlink::i386 {
| ^
llvm/include/llvm/ExecutionEngine/JITLink/i386.h:19:26: error: expected unqualified-id
19 | namespace llvm::jitlink::i386 {
| ^
```
The macro name 'i386' is obviously a historical bad choice, but since it existed long before llvm, llvm should either rename its namespace, or actively undefine the macro in `i386.h` (similar to e.g. https://github.com/google/swiftshader/blob/master/third_party/llvm-16.0/Android.bp#L1510).
This particular pull request implements the rename, but is meant to gauge opinions on how to solve this issue.
>From d5eb0a9484013197bd04ca40ead93fd5532a0c45 Mon Sep 17 00:00:00 2001
From: Dimitry Andric <dimitry at andric.com>
Date: Wed, 23 Apr 2025 23:18:17 +0200
Subject: [PATCH] [JITLink][i386] Avoid 'i386' name clashing with built-in
macro
When compiling llvm on an actual i386 platform, both clang and gcc
define a built-in macro `i386` to the value 1. This clashes with the
`llvm::jitlink::i386` namespace:
```
In file included from llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp:18:
llvm/include/llvm/ExecutionEngine/JITLink/i386.h:19:24: error: expected '{'
19 | namespace llvm::jitlink::i386 {
| ^
llvm/include/llvm/ExecutionEngine/JITLink/i386.h:19:26: error: expected unqualified-id
19 | namespace llvm::jitlink::i386 {
| ^
```
The macro name 'i386' is obviously a historical bad choice, but since it
existed long before llvm, llvm should either rename its namespace, or
actively undefine the macro in `i386.h` (similar to e.g.
https://github.com/google/swiftshader/blob/master/third_party/llvm-16.0/Android.bp#L1510).
This particular pull request implements the rename, but is meant to
gauge opinions on how to solve this issue.
---
.../llvm/ExecutionEngine/JITLink/i386.h | 36 +++++++--------
llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp | 44 +++++++++----------
llvm/lib/ExecutionEngine/JITLink/JITLink.cpp | 4 +-
llvm/lib/ExecutionEngine/JITLink/i386.cpp | 8 ++--
.../ExecutionEngine/JITLink/StubsTests.cpp | 4 +-
5 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/i386.h b/llvm/include/llvm/ExecutionEngine/JITLink/i386.h
index efe8182934dd7..f687d01c28de8 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/i386.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/i386.h
@@ -16,7 +16,7 @@
#include "llvm/ExecutionEngine/JITLink/JITLink.h"
#include "llvm/ExecutionEngine/JITLink/TableManager.h"
-namespace llvm::jitlink::i386 {
+namespace llvm::jitlink::i386_ {
/// Represets i386 fixups
enum EdgeKind_i386 : Edge::Kind {
@@ -184,7 +184,7 @@ const char *getEdgeKindName(Edge::Kind K);
/// Apply fixup expression for edge to block content.
inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
const Symbol *GOTSymbol) {
- using namespace i386;
+ using namespace i386_;
using namespace llvm::support;
char *BlockWorkingMem = B.getAlreadyMutableContent().data();
@@ -192,23 +192,23 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
auto FixupAddress = B.getAddress() + E.getOffset();
switch (E.getKind()) {
- case i386::None: {
+ case i386_::None: {
break;
}
- case i386::Pointer32: {
+ case i386_::Pointer32: {
uint32_t Value = E.getTarget().getAddress().getValue() + E.getAddend();
*(ulittle32_t *)FixupPtr = Value;
break;
}
- case i386::PCRel32: {
+ case i386_::PCRel32: {
int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
*(little32_t *)FixupPtr = Value;
break;
}
- case i386::Pointer16: {
+ case i386_::Pointer16: {
uint32_t Value = E.getTarget().getAddress().getValue() + E.getAddend();
if (LLVM_LIKELY(isUInt<16>(Value)))
*(ulittle16_t *)FixupPtr = Value;
@@ -217,7 +217,7 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
break;
}
- case i386::PCRel16: {
+ case i386_::PCRel16: {
int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
if (LLVM_LIKELY(isInt<16>(Value)))
*(little16_t *)FixupPtr = Value;
@@ -226,13 +226,13 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
break;
}
- case i386::Delta32: {
+ case i386_::Delta32: {
int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
*(little32_t *)FixupPtr = Value;
break;
}
- case i386::Delta32FromGOT: {
+ case i386_::Delta32FromGOT: {
assert(GOTSymbol && "No GOT section symbol");
int32_t Value =
E.getTarget().getAddress() - GOTSymbol->getAddress() + E.getAddend();
@@ -240,9 +240,9 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
break;
}
- case i386::BranchPCRel32:
- case i386::BranchPCRel32ToPtrJumpStub:
- case i386::BranchPCRel32ToPtrJumpStubBypassable: {
+ case i386_::BranchPCRel32:
+ case i386_::BranchPCRel32ToPtrJumpStub:
+ case i386_::BranchPCRel32ToPtrJumpStubBypassable: {
int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
*(little32_t *)FixupPtr = Value;
break;
@@ -328,14 +328,14 @@ class GOTTableManager : public TableManager<GOTTableManager> {
bool visitEdge(LinkGraph &G, Block *B, Edge &E) {
Edge::Kind KindToSet = Edge::Invalid;
switch (E.getKind()) {
- case i386::Delta32FromGOT: {
+ case i386_::Delta32FromGOT: {
// we need to make sure that the GOT section exists, but don't otherwise
// need to fix up this edge
getGOTSection(G);
return false;
}
- case i386::RequestGOTAndTransformToDelta32FromGOT:
- KindToSet = i386::Delta32FromGOT;
+ case i386_::RequestGOTAndTransformToDelta32FromGOT:
+ KindToSet = i386_::Delta32FromGOT;
break;
default:
return false;
@@ -374,7 +374,7 @@ class PLTTableManager : public TableManager<PLTTableManager> {
static StringRef getSectionName() { return "$__STUBS"; }
bool visitEdge(LinkGraph &G, Block *B, Edge &E) {
- if (E.getKind() == i386::BranchPCRel32 && !E.getTarget().isDefined()) {
+ if (E.getKind() == i386_::BranchPCRel32 && !E.getTarget().isDefined()) {
DEBUG_WITH_TYPE("jitlink", {
dbgs() << " Fixing " << G.getEdgeKindName(E.getKind()) << " edge at "
<< B->getFixupAddress(E) << " (" << B->getAddress() << " + "
@@ -382,7 +382,7 @@ class PLTTableManager : public TableManager<PLTTableManager> {
});
// Set the edge kind to Branch32ToPtrJumpStubBypassable to enable it to
// be optimized when the target is in-range.
- E.setKind(i386::BranchPCRel32ToPtrJumpStubBypassable);
+ E.setKind(i386_::BranchPCRel32ToPtrJumpStubBypassable);
E.setTarget(getEntryForTarget(G, E.getTarget()));
return true;
}
@@ -414,6 +414,6 @@ class PLTTableManager : public TableManager<PLTTableManager> {
/// target
Error optimizeGOTAndStubAccesses(LinkGraph &G);
-} // namespace llvm::jitlink::i386
+} // namespace llvm::jitlink::i386_
#endif // LLVM_EXECUTIONENGINE_JITLINK_I386_H
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp
index 4ce43c1962c84..600f44e56b6d4 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp
@@ -29,8 +29,8 @@ constexpr StringRef ELFGOTSymbolName = "_GLOBAL_OFFSET_TABLE_";
Error buildTables_ELF_i386(LinkGraph &G) {
LLVM_DEBUG(dbgs() << "Visiting edges in graph:\n");
- i386::GOTTableManager GOT;
- i386::PLTTableManager PLT(GOT);
+ i386_::GOTTableManager GOT;
+ i386_::PLTTableManager PLT(GOT);
visitExistingEdges(G, GOT, PLT);
return Error::success();
}
@@ -59,7 +59,7 @@ class ELFJITLinker_i386 : public JITLinker<ELFJITLinker_i386> {
if (Sym.getName() != nullptr &&
*Sym.getName() == ELFGOTSymbolName)
if (auto *GOTSection = G.findSectionByName(
- i386::GOTTableManager::getSectionName())) {
+ i386_::GOTTableManager::getSectionName())) {
GOTSymbol = &Sym;
return {*GOTSection, true};
}
@@ -79,7 +79,7 @@ class ELFJITLinker_i386 : public JITLinker<ELFJITLinker_i386> {
// record it, otherwise we'll create our own.
// If there's a GOT section but we didn't find an external GOT symbol...
if (auto *GOTSection =
- G.findSectionByName(i386::GOTTableManager::getSectionName())) {
+ G.findSectionByName(i386_::GOTTableManager::getSectionName())) {
// Check for an existing defined symbol.
for (auto *Sym : GOTSection->symbols())
@@ -106,15 +106,15 @@ class ELFJITLinker_i386 : public JITLinker<ELFJITLinker_i386> {
}
Error applyFixup(LinkGraph &G, Block &B, const Edge &E) const {
- return i386::applyFixup(G, B, E, GOTSymbol);
+ return i386_::applyFixup(G, B, E, GOTSymbol);
}
};
template <typename ELFT>
class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder<ELFT> {
private:
- static Expected<i386::EdgeKind_i386> getRelocationKind(const uint32_t Type) {
- using namespace i386;
+ static Expected<i386_::EdgeKind_i386> getRelocationKind(const uint32_t Type) {
+ using namespace i386_;
switch (Type) {
case ELF::R_386_NONE:
return EdgeKind_i386::None;
@@ -179,7 +179,7 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder<ELFT> {
Base::GraphSymbols.size()),
inconvertibleErrorCode());
- Expected<i386::EdgeKind_i386> Kind = getRelocationKind(Rel.getType(false));
+ Expected<i386_::EdgeKind_i386> Kind = getRelocationKind(Rel.getType(false));
if (!Kind)
return Kind.takeError();
@@ -187,23 +187,23 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder<ELFT> {
int64_t Addend = 0;
switch (*Kind) {
- case i386::EdgeKind_i386::None:
+ case i386_::EdgeKind_i386::None:
break;
- case i386::EdgeKind_i386::Pointer32:
- case i386::EdgeKind_i386::PCRel32:
- case i386::EdgeKind_i386::RequestGOTAndTransformToDelta32FromGOT:
- case i386::EdgeKind_i386::Delta32:
- case i386::EdgeKind_i386::Delta32FromGOT:
- case i386::EdgeKind_i386::BranchPCRel32:
- case i386::EdgeKind_i386::BranchPCRel32ToPtrJumpStub:
- case i386::EdgeKind_i386::BranchPCRel32ToPtrJumpStubBypassable: {
+ case i386_::EdgeKind_i386::Pointer32:
+ case i386_::EdgeKind_i386::PCRel32:
+ case i386_::EdgeKind_i386::RequestGOTAndTransformToDelta32FromGOT:
+ case i386_::EdgeKind_i386::Delta32:
+ case i386_::EdgeKind_i386::Delta32FromGOT:
+ case i386_::EdgeKind_i386::BranchPCRel32:
+ case i386_::EdgeKind_i386::BranchPCRel32ToPtrJumpStub:
+ case i386_::EdgeKind_i386::BranchPCRel32ToPtrJumpStubBypassable: {
const char *FixupContent = BlockToFix.getContent().data() +
(FixupAddress - BlockToFix.getAddress());
Addend = *(const support::little32_t *)FixupContent;
break;
}
- case i386::EdgeKind_i386::Pointer16:
- case i386::EdgeKind_i386::PCRel16: {
+ case i386_::EdgeKind_i386::Pointer16:
+ case i386_::EdgeKind_i386::PCRel16: {
const char *FixupContent = BlockToFix.getContent().data() +
(FixupAddress - BlockToFix.getAddress());
Addend = *(const support::little16_t *)FixupContent;
@@ -215,7 +215,7 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder<ELFT> {
Edge GE(*Kind, Offset, *GraphSymbol, Addend);
LLVM_DEBUG({
dbgs() << " ";
- printEdge(dbgs(), BlockToFix, GE, i386::getEdgeKindName(*Kind));
+ printEdge(dbgs(), BlockToFix, GE, i386_::getEdgeKindName(*Kind));
dbgs() << "\n";
});
@@ -229,7 +229,7 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder<ELFT> {
Triple TT, SubtargetFeatures Features)
: ELFLinkGraphBuilder<ELFT>(Obj, std::move(SSP), std::move(TT),
std::move(Features), FileName,
- i386::getEdgeKindName) {}
+ i386_::getEdgeKindName) {}
};
Expected<std::unique_ptr<LinkGraph>>
@@ -273,7 +273,7 @@ void link_ELF_i386(std::unique_ptr<LinkGraph> G,
Config.PostPrunePasses.push_back(buildTables_ELF_i386);
// Add GOT/Stubs optimizer pass.
- Config.PreFixupPasses.push_back(i386::optimizeGOTAndStubAccesses);
+ Config.PreFixupPasses.push_back(i386_::optimizeGOTAndStubAccesses);
}
if (auto Err = Ctx->modifyPassConfig(*G, Config))
return Ctx->notifyFailed(std::move(Err));
diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
index e1209e1e95496..e682ee0ca1983 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
@@ -466,7 +466,7 @@ AnonymousPointerCreator getAnonymousPointerCreator(const Triple &TT) {
case Triple::x86_64:
return x86_64::createAnonymousPointer;
case Triple::x86:
- return i386::createAnonymousPointer;
+ return i386_::createAnonymousPointer;
case Triple::loongarch32:
case Triple::loongarch64:
return loongarch::createAnonymousPointer;
@@ -482,7 +482,7 @@ PointerJumpStubCreator getPointerJumpStubCreator(const Triple &TT) {
case Triple::x86_64:
return x86_64::createAnonymousPointerJumpStub;
case Triple::x86:
- return i386::createAnonymousPointerJumpStub;
+ return i386_::createAnonymousPointerJumpStub;
case Triple::loongarch32:
case Triple::loongarch64:
return loongarch::createAnonymousPointerJumpStub;
diff --git a/llvm/lib/ExecutionEngine/JITLink/i386.cpp b/llvm/lib/ExecutionEngine/JITLink/i386.cpp
index e984bb10983d0..90d5c610dc412 100644
--- a/llvm/lib/ExecutionEngine/JITLink/i386.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/i386.cpp
@@ -14,7 +14,7 @@
#define DEBUG_TYPE "jitlink"
-namespace llvm::jitlink::i386 {
+namespace llvm::jitlink::i386_ {
const char *getEdgeKindName(Edge::Kind K) {
switch (K) {
@@ -55,7 +55,7 @@ Error optimizeGOTAndStubAccesses(LinkGraph &G) {
for (auto *B : G.blocks())
for (auto &E : B->edges()) {
- if (E.getKind() == i386::BranchPCRel32ToPtrJumpStubBypassable) {
+ if (E.getKind() == i386_::BranchPCRel32ToPtrJumpStubBypassable) {
auto &StubBlock = E.getTarget().getBlock();
assert(StubBlock.getSize() == sizeof(PointerJumpStubContent) &&
"Stub block should be stub sized");
@@ -74,7 +74,7 @@ Error optimizeGOTAndStubAccesses(LinkGraph &G) {
int64_t Displacement = TargetAddr - EdgeAddr + 4;
if (isInt<32>(Displacement)) {
- E.setKind(i386::BranchPCRel32);
+ E.setKind(i386_::BranchPCRel32);
E.setTarget(GOTTarget);
LLVM_DEBUG({
dbgs() << " Replaced stub branch with direct branch:\n ";
@@ -88,4 +88,4 @@ Error optimizeGOTAndStubAccesses(LinkGraph &G) {
return Error::success();
}
-} // namespace llvm::jitlink::i386
+} // namespace llvm::jitlink::i386_
diff --git a/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp b/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp
index 8a53d0a560ba3..95d9e219f3899 100644
--- a/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp
+++ b/llvm/unittests/ExecutionEngine/JITLink/StubsTests.cpp
@@ -102,13 +102,13 @@ TEST(StubsTest, StubsGeneration_i386) {
LinkGraph G("foo", std::make_shared<orc::SymbolStringPool>(),
Triple("i386-unknown-linux-gnu"), SubtargetFeatures(),
getGenericEdgeKindName);
- auto [PointerSym, StubSym] = GenerateStub(G, 4U, i386::Pointer32);
+ auto [PointerSym, StubSym] = GenerateStub(G, 4U, i386_::Pointer32);
EXPECT_EQ(std::distance(StubSym.getBlock().edges().begin(),
StubSym.getBlock().edges().end()),
1U);
auto &JumpEdge = *StubSym.getBlock().edges().begin();
- EXPECT_EQ(JumpEdge.getKind(), i386::Pointer32);
+ EXPECT_EQ(JumpEdge.getKind(), i386_::Pointer32);
EXPECT_EQ(&JumpEdge.getTarget(), &PointerSym);
EXPECT_EQ(StubSym.getBlock().getContent(),
ArrayRef<char>(PointerJumpStubContent));
More information about the llvm-commits
mailing list