From 0fde03cb7fdc29bedf2068e5702c36fd600c64cc Mon Sep 17 00:00:00 2001 From: Tobias Gysi Date: Wed, 4 Jan 2023 17:04:14 +0100 Subject: [PATCH] [mlir][llvm] Modernize the import of LLVM IR globals. Return failure if the import of a global variable fails and add a test case to check the emitted error message. Additionally, convert the globals in iteration order and do not process them recursively when translating a constant expression referencing it. Additionally, use the module location rather unknown location. Reviewed By: Dinistro Differential Revision: https://reviews.llvm.org/D140966 --- mlir/include/mlir/Target/LLVMIR/ModuleImport.h | 21 ++++++-------- mlir/lib/Target/LLVMIR/ModuleImport.cpp | 33 ++++++++++------------ mlir/test/Target/LLVMIR/Import/global-variables.ll | 8 ++++++ mlir/test/Target/LLVMIR/Import/import-failure.ll | 13 ++++++++- 4 files changed, 44 insertions(+), 31 deletions(-) diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h index 2493cbd..05094dd 100644 --- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h +++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h @@ -75,8 +75,8 @@ public: /// Stores a mapping between an LLVM instruction and the imported MLIR /// operation if the operation returns no result. Asserts if the operation /// returns a result and should be added to valueMapping instead. - void mapNoResultOp(llvm::Instruction *inst, Operation *mlir) { - mapNoResultOp(inst) = mlir; + void mapNoResultOp(llvm::Instruction *llvm, Operation *mlir) { + mapNoResultOp(llvm) = mlir; } /// Provides write-once access to store the MLIR operation corresponding to @@ -140,13 +140,10 @@ public: /// Imports `func` into the current module. LogicalResult processFunction(llvm::Function *func); - /// Converts function attributes of LLVM Function \p func - /// into LLVM dialect attributes of LLVMFuncOp \p funcOp. + /// Converts function attributes of LLVM Function `func` into LLVM dialect + /// attributes of LLVMFuncOp `funcOp`. void processFunctionAttributes(llvm::Function *func, LLVMFuncOp funcOp); - /// Imports `globalVar` as a GlobalOp, creating it if it doesn't exist. - GlobalOp processGlobal(llvm::GlobalVariable *globalVar); - /// Sets the fastmath flags attribute for the imported operation `op` given /// the original instruction `inst`. Asserts if the operation does not /// implement the fastmath interface. @@ -165,6 +162,11 @@ private: constantInsertionOp = nullptr; } + /// Converts an LLVM global variable into an MLIR LLVM dialect global + /// operation if a conversion exists. Otherwise, returns failure. + LogicalResult convertGlobal(llvm::GlobalVariable *globalVar); + /// Imports the magic globals "global_ctors" and "global_dtors". + LogicalResult convertGlobalCtorsAndDtors(llvm::GlobalVariable *globalVar); /// Returns personality of `func` as a FlatSymbolRefAttr. FlatSymbolRefAttr getPersonalityAsAttr(llvm::Function *func); /// Imports `bb` into `block`, which must be initially empty. @@ -218,9 +220,6 @@ private: /// function entry block. FailureOr convertConstantExpr(llvm::Constant *constant); - /// Imports the magic globals "global_ctors" and "global_dtors". - LogicalResult convertGlobalCtorsAndDtors(llvm::GlobalVariable *globalVar); - /// Builder pointing at where the next instruction should be generated. OpBuilder builder; /// Block to insert the next constant into. @@ -248,8 +247,6 @@ private: /// operations for all operations that return no result. All operations that /// return a result have a valueMapping entry instead. DenseMap noResultOpMapping; - /// Uniquing map of GlobalVariables. - DenseMap globals; /// The stateful type translator (contains named structs). LLVM::TypeFromLLVMIRTranslator typeTranslator; /// Stateful debug information importer. diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp index 1c0ac6b..b7c3457 100644 --- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp @@ -338,9 +338,10 @@ LogicalResult ModuleImport::convertGlobals() { } continue; } - - if (!processGlobal(&globalVar)) - return failure(); + if (failed(convertGlobal(&globalVar))) { + return emitError(mlirModule.getLoc()) + << "unhandled global variable " << diag(globalVar); + } } return success(); } @@ -531,17 +532,13 @@ Attribute ModuleImport::getConstantAsAttr(llvm::Constant *value) { return nullptr; } -GlobalOp ModuleImport::processGlobal(llvm::GlobalVariable *globalVar) { - if (globals.count(globalVar)) - return globals[globalVar]; - +LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) { // Insert the global after the last one or at the start of the module. OpBuilder::InsertionGuard guard(builder); - if (!globalInsertionOp) { + if (!globalInsertionOp) builder.setInsertionPointToStart(mlirModule.getBody()); - } else { + else builder.setInsertionPointAfter(globalInsertionOp); - } Attribute valueAttr; if (globalVar->hasInitializer()) @@ -556,7 +553,7 @@ GlobalOp ModuleImport::processGlobal(llvm::GlobalVariable *globalVar) { } GlobalOp globalOp = builder.create( - UnknownLoc::get(context), type, globalVar->isConstant(), + mlirModule.getLoc(), type, globalVar->isConstant(), convertLinkageFromLLVM(globalVar->getLinkage()), globalVar->getName(), valueAttr, alignment, /*addr_space=*/globalVar->getAddressSpace(), /*dso_local=*/globalVar->isDSOLocal(), @@ -570,7 +567,7 @@ GlobalOp ModuleImport::processGlobal(llvm::GlobalVariable *globalVar) { FailureOr initializer = convertConstantExpr(globalVar->getInitializer()); if (failed(initializer)) - return {}; + return failure(); builder.create(globalOp.getLoc(), *initializer); } if (globalVar->hasAtLeastLocalUnnamedAddr()) { @@ -580,7 +577,7 @@ GlobalOp ModuleImport::processGlobal(llvm::GlobalVariable *globalVar) { if (globalVar->hasSection()) globalOp.setSection(globalVar->getSection()); - return globals[globalVar] = globalOp; + return success(); } LogicalResult @@ -695,8 +692,9 @@ FailureOr ModuleImport::convertConstant(llvm::Constant *constant) { // Convert global variable accesses. if (auto *globalVar = dyn_cast(constant)) { - return builder.create(loc, processGlobal(globalVar)) - .getResult(); + Type type = convertType(globalVar->getType()); + auto symbolRef = FlatSymbolRefAttr::get(context, globalVar->getName()); + return builder.create(loc, type, symbolRef).getResult(); } // Convert constant expressions. @@ -771,11 +769,10 @@ FailureOr ModuleImport::convertConstantExpr(llvm::Constant *constant) { // Insert the constant after the last one or at the start or the entry block. OpBuilder::InsertionGuard guard(builder); - if (!constantInsertionOp) { + if (!constantInsertionOp) builder.setInsertionPointToStart(constantInsertionBlock); - } else { + else builder.setInsertionPointAfter(constantInsertionOp); - } // Convert all constants of the expression and add them to `valueMapping`. SetVector constantsToConvert = diff --git a/mlir/test/Target/LLVMIR/Import/global-variables.ll b/mlir/test/Target/LLVMIR/Import/global-variables.ll index 3bf37da..3e6e810 100644 --- a/mlir/test/Target/LLVMIR/Import/global-variables.ll +++ b/mlir/test/Target/LLVMIR/Import/global-variables.ll @@ -12,10 +12,18 @@ ; CHECK-SAME: {addr_space = 0 : i32, alignment = 8 : i64} : f64 @global_float = external global double, align 8 +; CHECK: llvm.mlir.global internal constant @address_before +; CHECK: = llvm.mlir.addressof @global_int : !llvm.ptr +@address_before = internal constant i32* @global_int + ; CHECK: llvm.mlir.global external @global_int ; CHECK-SAME: {addr_space = 0 : i32, alignment = 8 : i64} : i32 @global_int = external global i32, align 8 +; CHECK: llvm.mlir.global internal constant @address_after +; CHECK: = llvm.mlir.addressof @global_int : !llvm.ptr +@address_after = internal constant i32* @global_int + ; CHECK: llvm.mlir.global internal @global_string("hello world") @global_string = internal global [11 x i8] c"hello world" diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll index cba4173..c4dffe9 100644 --- a/mlir/test/Target/LLVMIR/Import/import-failure.ll +++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll @@ -27,6 +27,17 @@ bb1: ; // ----- +; CHECK: error: unhandled constant i8* blockaddress(@unhandled_global, %bb1) +; CHECK: error: unhandled global variable @private = private global i8* blockaddress(@unhandled_global, %bb1) +@private = private global i8* blockaddress(@unhandled_global, %bb1) + +define void @unhandled_global() { +bb1: + ret void +} + +; // ----- + declare void @llvm.gcroot(ptr %arg0, ptr %arg1) ; CHECK: error: unhandled intrinsic call void @llvm.gcroot(ptr %arg0, ptr %arg1) @@ -63,7 +74,7 @@ define void @dropped_instruction(i64 %arg1) { !llvm.module.flags = !{!0} !0 = !{i32 2, !"Debug Info Version", i32 3} !1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2) -!2 = !DIFile(filename: "debug-info.ll", directory: "/") +!2 = !DIFile(filename: "import-failure.ll", directory: "/") !3 = !DILocalVariable(scope: !4, name: "arg", file: !2, line: 1, arg: 1, align: 32); !4 = distinct !DISubprogram(name: "intrinsic", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1) !5 = !DILocation(line: 1, column: 2, scope: !4) -- 2.7.4