[llvm] [X86Backend] Use GetExternalSymbolSymbol for MO_ExternalSymbol. (PR #133352)
weiwei chen via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 29 07:40:45 PDT 2025
https://github.com/weiweichen updated https://github.com/llvm/llvm-project/pull/133352
>From 859ed81810432bf419ab661e31cc3256b3e3564f Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 28 Mar 2025 02:11:05 +0000
Subject: [PATCH 1/6] Use GetExternalSymbolSymbol for MO_ExternalSymbol.
---
llvm/lib/Target/X86/X86MCInstLower.cpp | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index 07f99dbba6a36..599861bb6fb45 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -192,8 +192,15 @@ MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
}
Name += Suffix;
- if (!Sym)
- Sym = Ctx.getOrCreateSymbol(Name);
+ if (!Sym) {
+ MCSymbol* S = Ctx.lookupSymbol(Name);
+ // If new MCSymbol needs to be created for
+ // MachineOperand::MO_ExternalSymbol, create is as an external symbol.
+ if(!S && MO.getType() == MachineOperand::MO_ExternalSymbol)
+ Sym = AsmPrinter.GetExternalSymbolSymbol(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.
@@ -349,12 +356,8 @@ MCOperand X86MCInstLower::LowerMachineOperand(const MachineInstr *MI,
return MCOperand::createImm(MO.getImm());
case MachineOperand::MO_MachineBasicBlock:
case MachineOperand::MO_GlobalAddress:
- return LowerSymbolOperand(MO, GetSymbolFromOperand(MO));
case MachineOperand::MO_ExternalSymbol: {
- MCSymbol *Sym = GetSymbolFromOperand(MO);
- Sym->setExternal(true);
- return LowerSymbolOperand(MO, Sym);
- }
+ return LowerSymbolOperand(MO, GetSymbolFromOperand(MO));
case MachineOperand::MO_MCSymbol:
return LowerSymbolOperand(MO, MO.getMCSymbol());
case MachineOperand::MO_JumpTableIndex:
>From ec3e34e3161b68c4f8e5a1a728d536e4f903de93 Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 28 Mar 2025 02:31:26 +0000
Subject: [PATCH 2/6] Fix formatting.
---
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 e536b0d411b8b..b3383a63fb519 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -193,10 +193,10 @@ MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
Name += Suffix;
if (!Sym) {
- MCSymbol* S = Ctx.lookupSymbol(Name);
+ MCSymbol *S = Ctx.lookupSymbol(Name);
// If new MCSymbol needs to be created for
// MachineOperand::MO_ExternalSymbol, create is as an external symbol.
- if(!S && MO.getType() == MachineOperand::MO_ExternalSymbol)
+ if (!S && MO.getType() == MachineOperand::MO_ExternalSymbol)
Sym = AsmPrinter.GetExternalSymbolSymbol(Name);
else
Sym = Ctx.getOrCreateSymbol(Name);
>From cf82df020926ac0dc0b7d7bfa3fcd88ed447884c Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 28 Mar 2025 02:59:45 +0000
Subject: [PATCH 3/6] Make change more explicit.
---
llvm/lib/Target/X86/X86MCInstLower.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index b3383a63fb519..927cda9cb3eba 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -193,11 +193,11 @@ MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
Name += Suffix;
if (!Sym) {
- MCSymbol *S = Ctx.lookupSymbol(Name);
// If new MCSymbol needs to be created for
- // MachineOperand::MO_ExternalSymbol, create is as an external symbol.
- if (!S && MO.getType() == MachineOperand::MO_ExternalSymbol)
- Sym = AsmPrinter.GetExternalSymbolSymbol(Name);
+ // MachineOperand::MO_ExternalSymbol, create it as an symbol
+ // in AsmPrinter's OutContext.
+ if (MO.isSymbol())
+ Sym = AsmPrinter.OutContext.getOrCreateSymbol(Name);
else
Sym = Ctx.getOrCreateSymbol(Name);
}
>From fe9668bea831e31b1a7058722245e6826b819850 Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Fri, 28 Mar 2025 03:02:07 +0000
Subject: [PATCH 4/6] Fix English.
---
llvm/lib/Target/X86/X86MCInstLower.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index 927cda9cb3eba..c93a2879a057f 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -194,7 +194,7 @@ MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
Name += Suffix;
if (!Sym) {
// If new MCSymbol needs to be created for
- // MachineOperand::MO_ExternalSymbol, create it as an symbol
+ // MachineOperand::MO_ExternalSymbol, create it as a symbol
// in AsmPrinter's OutContext.
if (MO.isSymbol())
Sym = AsmPrinter.OutContext.getOrCreateSymbol(Name);
>From e395289279818fd45a6f897d26d292df9149822c Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Sat, 29 Mar 2025 10:11:34 -0400
Subject: [PATCH 5/6] Add unittest.
---
llvm/unittests/CodeGen/CMakeLists.txt | 1 +
llvm/unittests/CodeGen/X86MCInstLowerTest.cpp | 180 ++++++++++++++++++
2 files changed, 181 insertions(+)
create mode 100644 llvm/unittests/CodeGen/X86MCInstLowerTest.cpp
diff --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index 4f580e7539f4d..a972dc32c40a2 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -47,6 +47,7 @@ add_llvm_unittest(CodeGenTests
TargetOptionsTest.cpp
TestAsmPrinter.cpp
MLRegAllocDevelopmentFeatures.cpp
+ X86MCInstLowerTest.cpp
)
add_subdirectory(GlobalISel)
diff --git a/llvm/unittests/CodeGen/X86MCInstLowerTest.cpp b/llvm/unittests/CodeGen/X86MCInstLowerTest.cpp
new file mode 100644
index 0000000000000..9909bc9ffad4a
--- /dev/null
+++ b/llvm/unittests/CodeGen/X86MCInstLowerTest.cpp
@@ -0,0 +1,180 @@
+//===- llvm/unittest/CodeGen/AArch64SelectionDAGTest.cpp
+//-------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../lib/Target/X86/X86ISelLowering.h"
+#include "TestAsmPrinter.h"
+#include "llvm/Analysis/MemoryLocation.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/CodeGen/AsmPrinter.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/SelectionDAG.h"
+#include "llvm/CodeGen/TargetLowering.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/IR/LegacyPassManager.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Module.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/KnownBits.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/Target/TargetLoweringObjectFile.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace llvm {
+
+class X86MCInstLowerTest : public testing::Test {
+protected:
+ static void SetUpTestCase() {
+ LLVMInitializeX86TargetInfo();
+ LLVMInitializeX86TargetMC();
+ LLVMInitializeX86Target();
+ LLVMInitializeX86AsmPrinter();
+ }
+
+ // Function to setup codegen pipeline and returns the AsmPrinter.
+ AsmPrinter *addPassesToEmitFile(llvm::legacy::PassManagerBase &PM,
+ llvm::raw_pwrite_stream &Out,
+ llvm::CodeGenFileType FileType,
+ llvm::MachineModuleInfoWrapperPass *MMIWP) {
+ TargetPassConfig *PassConfig = TM->createPassConfig(PM);
+
+ PassConfig->setDisableVerify(true);
+ PM.add(PassConfig);
+ PM.add(MMIWP);
+
+ if (PassConfig->addISelPasses())
+ return nullptr;
+
+ PassConfig->addMachinePasses();
+ PassConfig->setInitialized();
+
+ MC.reset(new MCContext(TM->getTargetTriple(), TM->getMCAsmInfo(),
+ TM->getMCRegisterInfo(), TM->getMCSubtargetInfo()));
+ MC->setObjectFileInfo(TM->getObjFileLowering());
+ TM->getObjFileLowering()->Initialize(*MC, *TM);
+ MC->setObjectFileInfo(TM->getObjFileLowering());
+
+ // Use a new MCContext for AsmPrinter for testing.
+ // AsmPrinter.OutContext will be different from
+ // MachineFunction's MCContext in MMIWP.
+ Expected<std::unique_ptr<MCStreamer>> MCStreamerOrErr =
+ TM->createMCStreamer(Out, nullptr, FileType, *MC);
+
+ if (auto Err = MCStreamerOrErr.takeError())
+ return nullptr;
+
+ AsmPrinter *Printer =
+ TM->getTarget().createAsmPrinter(*TM, std::move(*MCStreamerOrErr));
+
+ if (!Printer)
+ return nullptr;
+
+ PM.add(Printer);
+
+ return Printer;
+ }
+
+ void SetUp() override {
+ // Module to compile.
+ const char *FooStr = R""""(
+ @G = external global i32
+
+ define i32 @foo() {
+ %1 = load i32, i32* @G; load the global variable
+ %2 = call i32 @f()
+ %3 = mul i32 %1, %2
+ ret i32 %3
+ }
+
+ declare i32 @f() #0
+ )"""";
+ StringRef AssemblyF(FooStr);
+
+ // Get target triple for X86_64
+ Triple TargetTriple("x86_64--");
+ std::string Error;
+ const Target *T = TargetRegistry::lookupTarget("", TargetTriple, Error);
+ if (!T)
+ GTEST_SKIP();
+
+ // Get TargetMachine.
+ // Use Reloc::Model::PIC_ and CodeModel::Model::Large
+ // to get GOT during codegen as MO_ExternalSymbol.
+ TargetOptions Options;
+ TM = std::unique_ptr<TargetMachine>(T->createTargetMachine(
+ TargetTriple, "", "", Options, Reloc::Model::PIC_,
+ CodeModel::Model::Large, CodeGenOptLevel::Default));
+ if (!TM)
+ GTEST_SKIP();
+
+ SMDiagnostic SMError;
+
+ // Parse the module.
+ M = parseAssemblyString(AssemblyF, SMError, Context);
+ if (!M)
+ report_fatal_error(SMError.getMessage());
+ M->setDataLayout(TM->createDataLayout());
+
+ // Get llvm::Function from M
+ Foo = M->getFunction("foo");
+ if (!Foo)
+ report_fatal_error("foo?");
+
+ // Prepare the MCContext for codegen M.
+ // MachineFunction for Foo will have this MCContext.
+ MCFoo.reset(new MCContext(TargetTriple, TM->getMCAsmInfo(),
+ TM->getMCRegisterInfo(),
+ TM->getMCSubtargetInfo()));
+ MCFoo->setObjectFileInfo(TM->getObjFileLowering());
+ TM->getObjFileLowering()->Initialize(*MCFoo, *TM);
+ MCFoo->setObjectFileInfo(TM->getObjFileLowering());
+ }
+
+ LLVMContext Context;
+ std::unique_ptr<TargetMachine> TM;
+ std::unique_ptr<Module> M;
+
+ std::unique_ptr<MCContext> MC;
+ std::unique_ptr<MCContext> MCFoo;
+
+ Function *Foo;
+ std::unique_ptr<MachineFunction> MFFoo;
+};
+
+TEST_F(X86MCInstLowerTest, moExternalSymbol_MCSYMBOL) {
+
+ MachineModuleInfoWrapperPass *MMIWP =
+ new MachineModuleInfoWrapperPass(TM.get(), &*MCFoo);
+
+ legacy::PassManager PassMgrF;
+ SmallString<1024> Buf;
+ llvm::raw_svector_ostream OS(Buf);
+ AsmPrinter *Printer =
+ addPassesToEmitFile(PassMgrF, OS, CodeGenFileType::AssemblyFile, MMIWP);
+ PassMgrF.run(*M);
+
+ // Check GOT MCSymbol is from Printer.OutContext.
+ MCSymbol *GOTPrinterPtr =
+ Printer->OutContext.lookupSymbol("_GLOBAL_OFFSET_TABLE_");
+
+ // Check GOT MCSymbol is NOT from MachineFunction's MCContext.
+ MCSymbol *GOTMFCtxPtr =
+ MMIWP->getMMI().getMachineFunction(*Foo)->getContext().lookupSymbol(
+ "_GLOBAL_OFFSET_TABLE_");
+
+ EXPECT_NE(GOTPrinterPtr, nullptr);
+ EXPECT_EQ(GOTMFCtxPtr, nullptr);
+}
+
+} // end namespace llvm
>From fc41f66a14c1d476942ca190941d78d6a8ae1581 Mon Sep 17 00:00:00 2001
From: Weiwei Chen <weiwei.chen at modular.com>
Date: Sat, 29 Mar 2025 10:40:19 -0400
Subject: [PATCH 6/6] Fix M68k backend.
---
llvm/lib/Target/M68k/M68kMCInstLower.cpp | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/M68k/M68kMCInstLower.cpp b/llvm/lib/Target/M68k/M68kMCInstLower.cpp
index 957c0f9d3da82..688061877017f 100644
--- a/llvm/lib/Target/M68k/M68kMCInstLower.cpp
+++ b/llvm/lib/Target/M68k/M68kMCInstLower.cpp
@@ -65,8 +65,12 @@ M68kMCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
}
Name += Suffix;
- if (!Sym)
- Sym = Ctx.getOrCreateSymbol(Name);
+ if (!Sym) {
+ if (MO.isSymbol())
+ Sym = AsmPrinter.OutContext.getOrCreateSymbol(Name);
+ else
+ Sym = Ctx.getOrCreateSymbol(Name);
+ }
return Sym;
}
More information about the llvm-commits
mailing list