[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

Author: gysit

mlir/include/mlir/Target/LLVMIR/ModuleImport.h

@@ -75,8 +75,8 @@ class ModuleImport {
   /// 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 @@ class ModuleImport {
   /// 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 @@ class ModuleImport {
     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 @@ class ModuleImport {
   /// function entry block.
   FailureOr<Value> 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 @@ class ModuleImport {
   /// operations for all operations that return no result. All operations that
   /// return a result have a valueMapping entry instead.
   DenseMap<llvm::Instruction *, Operation *> noResultOpMapping;
-  /// Uniquing map of GlobalVariables.
-  DenseMap<llvm::GlobalVariable *, GlobalOp> globals;
   /// The stateful type translator (contains named structs).
   LLVM::TypeFromLLVMIRTranslator typeTranslator;
   /// Stateful debug information importer.

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<GlobalOp>(
-      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<Value> initializer =
         convertConstantExpr(globalVar->getInitializer());
     if (failed(initializer))
-      return {};
+      return failure();
     builder.create<ReturnOp>(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<Value> ModuleImport::convertConstant(llvm::Constant *constant) {
 
   // Convert global variable accesses.
   if (auto *globalVar = dyn_cast<llvm::GlobalVariable>(constant)) {
-    return builder.create<AddressOfOp>(loc, processGlobal(globalVar))
-        .getResult();
+    Type type = convertType(globalVar->getType());
+    auto symbolRef = FlatSymbolRefAttr::get(context, globalVar->getName());
+    return builder.create<AddressOfOp>(loc, type, symbolRef).getResult();
   }
 
   // Convert constant expressions.
@@ -771,11 +769,10 @@ FailureOr<Value> 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<llvm::Constant *> constantsToConvert =

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<i32>
+@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<i32>
+@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"
 

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)