Skip to content

Commit

Permalink
[MLIR][LLVM] Fix nameless global import to support use before def case (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
Dinistro authored Oct 10, 2024
1 parent 06eb10d commit 54d3cf1
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 15 deletions.
4 changes: 4 additions & 0 deletions mlir/include/mlir/Target/LLVMIR/ModuleImport.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,10 @@ 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
getOrCreateNamelessSymbolName(llvm::GlobalVariable *globalVar);

/// Builder pointing at where the next instruction should be generated.
OpBuilder builder;
Expand Down
42 changes: 27 additions & 15 deletions mlir/lib/Target/LLVMIR/ModuleImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,24 @@ Attribute ModuleImport::getConstantAsAttr(llvm::Constant *constant) {
return {};
}

FlatSymbolRefAttr
ModuleImport::getOrCreateNamelessSymbolName(llvm::GlobalVariable *globalVar) {
assert(globalVar->getName().empty() &&
"expected to work with a nameless global");
auto [it, success] = namelessGlobals.try_emplace(globalVar);
if (!success)
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);
it->getSecond() = 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);
Expand Down Expand Up @@ -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 = getOrCreateNamelessSymbolName(globalVar).getValue();

GlobalOp globalOp = builder.create<GlobalOp>(
mlirModule.getLoc(), type, globalVar->isConstant(),
convertLinkageFromLLVM(globalVar->getLinkage()), StringRef(globalName),
Expand Down Expand Up @@ -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 =
getOrCreateNamelessSymbolName(cast<llvm::GlobalVariable>(globalObj));
else
symbolRef = FlatSymbolRefAttr::get(context, globalName);
return builder.create<AddressOfOp>(loc, type, symbolRef).getResult();
Expand Down
11 changes: 11 additions & 0 deletions mlir/test/Target/LLVMIR/Import/global-variables.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit 54d3cf1

Please sign in to comment.