[MLIR][LLVM] Fix nameless global import to support use before def case #111797
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-mlir-llvm Author: Christian Ulmann (Dinistro) ChangesThis 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. Full diff: https://github.com/llvm/llvm-project/pull/111797.diff 3 Files Affected:
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
+@reference = internal constant ptr @0
+
+; CHECK: llvm.mlir.global private unnamed_addr constant @mlir.llvm.nameless_global_0("0\00")
+@0 = private unnamed_addr constant [2 x i8] c"0\00"
|
|
@llvm/pr-subscribers-mlir Author: Christian Ulmann (Dinistro) ChangesThis 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. Full diff: https://github.com/llvm/llvm-project/pull/111797.diff 3 Files Affected:
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
+@reference = internal constant ptr @0
+
+; CHECK: llvm.mlir.global private unnamed_addr constant @mlir.llvm.nameless_global_0("0\00")
+@0 = private unnamed_addr constant [2 x i8] c"0\00"
|
ivanradanov
approved these changes
Oct 10, 2024
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
llvm#111797) 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.