[Mlir-commits] [mlir] [MLIR][LLVM] Fix nameless global import to support use before def case (PR #111797)
Christian Ulmann
llvmlistbot at llvm.org
Thu Oct 10 01:00:43 PDT 2024
https://github.com/Dinistro updated https://github.com/llvm/llvm-project/pull/111797
>From 3959cc948cbdb5e4e5b803aa586ef0b0ffea7c78 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Thu, 10 Oct 2024 07:09:07 +0000
Subject: [PATCH 1/2] [MLIR][LLVM] Fix nameless global import to support use
before def case
This commit fixes a bug in the import of nameless globals. Before this
change, the fake symbol names were only generated during the
transformation of the definition. This caused issues when the symbol was
used before it was defined.
---
.../include/mlir/Target/LLVMIR/ModuleImport.h | 3 ++
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 42 ++++++++++++-------
.../Target/LLVMIR/Import/global-variables.ll | 11 +++++
3 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 11f62c283d6a96..e4b2e51bdbe846 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -355,6 +355,9 @@ class ModuleImport {
/// and stores a mapping from the struct to the symbol pointing to the
/// translated operation.
void processComdat(const llvm::Comdat *comdat);
+ /// Returns a symbol name for a nameless global. MLIR, in contrast to LLVM,
+ /// always requires a symbol name.
+ FlatSymbolRefAttr getOrCreateFakeSymbolName(llvm::GlobalVariable *globalVar);
/// Builder pointing at where the next instruction should be generated.
OpBuilder builder;
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index a26cbdae561373..f979fbb7b837fa 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -874,6 +874,24 @@ Attribute ModuleImport::getConstantAsAttr(llvm::Constant *constant) {
return {};
}
+FlatSymbolRefAttr
+ModuleImport::getOrCreateFakeSymbolName(llvm::GlobalVariable *globalVar) {
+ assert(globalVar->getName().empty() &&
+ "expected to work with a nameless global");
+ auto it = namelessGlobals.find(globalVar);
+ if (it != namelessGlobals.end())
+ return it->second;
+
+ // Make sure the symbol name does not clash with an existing symbol.
+ SmallString<128> globalName = SymbolTable::generateSymbolName<128>(
+ getNamelessGlobalPrefix(),
+ [this](StringRef newName) { return llvmModule->getNamedValue(newName); },
+ namelessGlobalId);
+ auto symbolRef = FlatSymbolRefAttr::get(context, globalName);
+ namelessGlobals[globalVar] = symbolRef;
+ return symbolRef;
+}
+
LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
// Insert the global after the last one or at the start of the module.
OpBuilder::InsertionGuard guard(builder);
@@ -907,17 +925,10 @@ LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
// Workaround to support LLVM's nameless globals. MLIR, in contrast to LLVM,
// always requires a symbol name.
- SmallString<128> globalName(globalVar->getName());
- if (globalName.empty()) {
- // Make sure the symbol name does not clash with an existing symbol.
- globalName = SymbolTable::generateSymbolName<128>(
- getNamelessGlobalPrefix(),
- [this](StringRef newName) {
- return llvmModule->getNamedValue(newName);
- },
- namelessGlobalId);
- namelessGlobals[globalVar] = FlatSymbolRefAttr::get(context, globalName);
- }
+ StringRef globalName = globalVar->getName();
+ if (globalName.empty())
+ globalName = getOrCreateFakeSymbolName(globalVar).getValue();
+
GlobalOp globalOp = builder.create<GlobalOp>(
mlirModule.getLoc(), type, globalVar->isConstant(),
convertLinkageFromLLVM(globalVar->getLinkage()), StringRef(globalName),
@@ -1100,13 +1111,14 @@ FailureOr<Value> ModuleImport::convertConstant(llvm::Constant *constant) {
}
// Convert global variable accesses.
- if (auto *globalVar = dyn_cast<llvm::GlobalObject>(constant)) {
- Type type = convertType(globalVar->getType());
- StringRef globalName = globalVar->getName();
+ if (auto *globalObj = dyn_cast<llvm::GlobalObject>(constant)) {
+ Type type = convertType(globalObj->getType());
+ StringRef globalName = globalObj->getName();
FlatSymbolRefAttr symbolRef;
// Empty names are only allowed for global variables.
if (globalName.empty())
- symbolRef = namelessGlobals[cast<llvm::GlobalVariable>(globalVar)];
+ symbolRef =
+ getOrCreateFakeSymbolName(cast<llvm::GlobalVariable>(globalObj));
else
symbolRef = FlatSymbolRefAttr::get(context, globalName);
return builder.create<AddressOfOp>(loc, type, symbolRef).getResult();
diff --git a/mlir/test/Target/LLVMIR/Import/global-variables.ll b/mlir/test/Target/LLVMIR/Import/global-variables.ll
index d6d2492b4f1770..879e9135fe4ece 100644
--- a/mlir/test/Target/LLVMIR/Import/global-variables.ll
+++ b/mlir/test/Target/LLVMIR/Import/global-variables.ll
@@ -330,3 +330,14 @@ declare void @"mlir.llvm.nameless_global_2"()
!5 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
!6 = !{}
!7 = !{i32 2, !"Debug Info Version", i32 3}
+
+; // -----
+
+; Verify that unnamed globals can also be referenced before they are defined.
+
+; CHECK: llvm.mlir.global internal constant @reference()
+; CHECK: llvm.mlir.addressof @mlir.llvm.nameless_global_0 : !llvm.ptr
+ at reference = internal constant ptr @0
+
+; CHECK: llvm.mlir.global private unnamed_addr constant @mlir.llvm.nameless_global_0("0\00")
+ at 0 = private unnamed_addr constant [2 x i8] c"0\00"
>From 2ca0156d48171b7064069809f5ce3fe1f6ae67b1 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Thu, 10 Oct 2024 08:00:30 +0000
Subject: [PATCH 2/2] address review comment
---
mlir/include/mlir/Target/LLVMIR/ModuleImport.h | 3 ++-
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 12 ++++++------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index e4b2e51bdbe846..436675793062eb 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -357,7 +357,8 @@ class ModuleImport {
void processComdat(const llvm::Comdat *comdat);
/// Returns a symbol name for a nameless global. MLIR, in contrast to LLVM,
/// always requires a symbol name.
- FlatSymbolRefAttr getOrCreateFakeSymbolName(llvm::GlobalVariable *globalVar);
+ FlatSymbolRefAttr
+ getOrCreateNamelessSymbolName(llvm::GlobalVariable *globalVar);
/// Builder pointing at where the next instruction should be generated.
OpBuilder builder;
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index f979fbb7b837fa..4ff1f1135b0a88 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -875,11 +875,11 @@ Attribute ModuleImport::getConstantAsAttr(llvm::Constant *constant) {
}
FlatSymbolRefAttr
-ModuleImport::getOrCreateFakeSymbolName(llvm::GlobalVariable *globalVar) {
+ModuleImport::getOrCreateNamelessSymbolName(llvm::GlobalVariable *globalVar) {
assert(globalVar->getName().empty() &&
"expected to work with a nameless global");
- auto it = namelessGlobals.find(globalVar);
- if (it != namelessGlobals.end())
+ auto [it, success] = namelessGlobals.try_emplace(globalVar);
+ if (!success)
return it->second;
// Make sure the symbol name does not clash with an existing symbol.
@@ -888,7 +888,7 @@ ModuleImport::getOrCreateFakeSymbolName(llvm::GlobalVariable *globalVar) {
[this](StringRef newName) { return llvmModule->getNamedValue(newName); },
namelessGlobalId);
auto symbolRef = FlatSymbolRefAttr::get(context, globalName);
- namelessGlobals[globalVar] = symbolRef;
+ it->getSecond() = symbolRef;
return symbolRef;
}
@@ -927,7 +927,7 @@ LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
// always requires a symbol name.
StringRef globalName = globalVar->getName();
if (globalName.empty())
- globalName = getOrCreateFakeSymbolName(globalVar).getValue();
+ globalName = getOrCreateNamelessSymbolName(globalVar).getValue();
GlobalOp globalOp = builder.create<GlobalOp>(
mlirModule.getLoc(), type, globalVar->isConstant(),
@@ -1118,7 +1118,7 @@ FailureOr<Value> ModuleImport::convertConstant(llvm::Constant *constant) {
// Empty names are only allowed for global variables.
if (globalName.empty())
symbolRef =
- getOrCreateFakeSymbolName(cast<llvm::GlobalVariable>(globalObj));
+ getOrCreateNamelessSymbolName(cast<llvm::GlobalVariable>(globalObj));
else
symbolRef = FlatSymbolRefAttr::get(context, globalName);
return builder.create<AddressOfOp>(loc, type, symbolRef).getResult();
More information about the Mlir-commits
mailing list