[mlir] don't drop undef initializers in translation to LLVM IR

LLVM IR allows globals with external linkage to have initializers, including
undef. The translation was incorrectly using undef as a indicator that the
initializer should be ignored in translation, leading to the impossibility to
create an external global with an explicit undef initializer. Fix this and use
nullptr as a marker instead.

Reviewed By: wsmoses

Differential Revision: https://reviews.llvm.org/D105631

Author: ftynse

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

@@ -442,11 +442,13 @@ static Block &getModuleBody(Operation *module) {
 }
 
 /// A helper method to decide if a constant must not be set as a global variable
-/// initializer.
+/// initializer. For an external linkage variable, the variable with an
+/// initializer is considered externally visible and defined in this module, the
+/// variable without an initializer is externally available and is defined
+/// elsewhere.
 static bool shouldDropGlobalInitializer(llvm::GlobalValue::LinkageTypes linkage,
                                         llvm::Constant *cst) {
-  return (linkage == llvm::GlobalVariable::ExternalLinkage &&
-          isa<llvm::UndefValue>(cst)) ||
+  return (linkage == llvm::GlobalVariable::ExternalLinkage && !cst) ||
          linkage == llvm::GlobalVariable::ExternalWeakLinkage;
 }
 
@@ -455,7 +457,7 @@ static bool shouldDropGlobalInitializer(llvm::GlobalValue::LinkageTypes linkage,
 LogicalResult ModuleTranslation::convertGlobals() {
   for (auto op : getModuleBody(mlirModule).getOps<LLVM::GlobalOp>()) {
     llvm::Type *type = convertType(op.getType());
-    llvm::Constant *cst = llvm::UndefValue::get(type);
+    llvm::Constant *cst = nullptr;
     if (op.getValueOrNull()) {
       // String attributes are treated separately because they cannot appear as
       // in-function constants and are thus not supported by getLLVMConstant.
@@ -471,10 +473,18 @@ LogicalResult ModuleTranslation::convertGlobals() {
 
     auto linkage = convertLinkageToLLVM(op.linkage());
     auto addrSpace = op.addr_space();
+
+    // LLVM IR requires constant with linkage other than external or weak
+    // external to have initializers. If MLIR does not provide an initializer,
+    // default to undef.
+    bool dropInitializer = shouldDropGlobalInitializer(linkage, cst);
+    if (!dropInitializer && !cst)
+      cst = llvm::UndefValue::get(type);
+    else if (dropInitializer && cst)
+      cst = nullptr;
+
     auto *var = new llvm::GlobalVariable(
-        *llvmModule, type, op.constant(), linkage,
-        shouldDropGlobalInitializer(linkage, cst) ? nullptr : cst,
-        op.sym_name(),
+        *llvmModule, type, op.constant(), linkage, cst, op.sym_name(),
         /*InsertBefore=*/nullptr, llvm::GlobalValue::NotThreadLocal, addrSpace);
 
     if (op.unnamed_addr().hasValue())

mlir/test/Target/LLVMIR/llvmir.mlir

@@ -33,6 +33,12 @@ llvm.mlir.global internal constant @string_const("foobar") : !llvm.array<6 x i8>
 // CHECK: @int_global_undef = internal global i64 undef
 llvm.mlir.global internal @int_global_undef() : i64
 
+// CHECK: @explicit_undef = global i32 undef
+llvm.mlir.global external @explicit_undef() : i32 {
+  %0 = llvm.mlir.undef : i32
+  llvm.return %0 : i32
+}
+
 // CHECK: @int_gep = internal constant i32* getelementptr (i32, i32* @i32_global, i32 2)
 llvm.mlir.global internal constant @int_gep() : !llvm.ptr<i32> {
   %addr = llvm.mlir.addressof @i32_global : !llvm.ptr<i32>