[Mlir-commits] [mlir] efa2d53 - [mlir] error out on unsupported attribute kinds in LLVM global translation

[Alex Zinenko]

Author: Alex Zinenko
Date: 2020-03-03T17:08:28+01:00
New Revision: efa2d533773163fafa182bd66003cf4527d46a0f

URL: https://github.com/llvm/llvm-project/commit/efa2d533773163fafa182bd66003cf4527d46a0f
DIFF: https://github.com/llvm/llvm-project/commit/efa2d533773163fafa182bd66003cf4527d46a0f.diff

LOG: [mlir] error out on unsupported attribute kinds in LLVM global translation

Some attribute kinds are not supported as "value" attributes of
`llvm.mlir.constant` when translating to LLVM IR. We were correctly reporting
an error, but continuing the translation using an "undef" value instead,
leading to a surprising mix of error messages and output IR. Abort the
translation after the error is reported.

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

Added: 
    

Modified: 
    mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
    mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
    mlir/test/Target/llvmir-invalid.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
index 5eda0f6bb30c..84ee37a8cc54 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -56,7 +56,8 @@ class ModuleTranslation {
       return nullptr;
 
     T translator(m, std::move(llvmModule));
-    translator.convertGlobals();
+    if (failed(translator.convertGlobals()))
+      return nullptr;
     if (failed(translator.convertFunctions()))
       return nullptr;
 
@@ -87,7 +88,7 @@ class ModuleTranslation {
   static LogicalResult checkSupportedModuleOps(Operation *m);
 
   LogicalResult convertFunctions();
-  void convertGlobals();
+  LogicalResult convertGlobals();
   LogicalResult convertOneFunction(LLVMFuncOp func);
   void connectPHINodes(LLVMFuncOp func);
   LogicalResult convertBlock(Block &bb, bool ignoreArguments);

diff  --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index cbcc2e644636..84985fc68651 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -421,7 +421,7 @@ LogicalResult ModuleTranslation::convertBlock(Block &bb, bool ignoreArguments) {
 
 /// Create named global variables that correspond to llvm.mlir.global
 /// definitions.
-void ModuleTranslation::convertGlobals() {
+LogicalResult ModuleTranslation::convertGlobals() {
   for (auto op : getModuleBody(mlirModule).getOps<LLVM::GlobalOp>()) {
     llvm::Type *type = op.getType().getUnderlyingType();
     llvm::Constant *cst = llvm::UndefValue::get(type);
@@ -432,17 +432,16 @@ void ModuleTranslation::convertGlobals() {
         cst = llvm::ConstantDataArray::getString(
             llvmModule->getContext(), strAttr.getValue(), /*AddNull=*/false);
         type = cst->getType();
-      } else {
-        cst = getLLVMConstant(type, op.getValueOrNull(), op.getLoc());
+      } else if (!(cst = getLLVMConstant(type, op.getValueOrNull(),
+                                         op.getLoc()))) {
+        return failure();
       }
     } else if (Block *initializer = op.getInitializerBlock()) {
       llvm::IRBuilder<> builder(llvmModule->getContext());
       for (auto &op : initializer->without_terminator()) {
         if (failed(convertOperation(op, builder)) ||
-            !isa<llvm::Constant>(valueMapping.lookup(op.getResult(0)))) {
-          emitError(op.getLoc(), "unemittable constant value");
-          return;
-        }
+            !isa<llvm::Constant>(valueMapping.lookup(op.getResult(0))))
+          return emitError(op.getLoc(), "unemittable constant value");
       }
       ReturnOp ret = cast<ReturnOp>(initializer->getTerminator());
       cst = cast<llvm::Constant>(valueMapping.lookup(ret.getOperand(0)));
@@ -460,6 +459,8 @@ void ModuleTranslation::convertGlobals() {
 
     globalsMapping.try_emplace(op, var);
   }
+
+  return success();
 }
 
 /// Get the SSA value passed to the current block from the terminator operation

diff  --git a/mlir/test/Target/llvmir-invalid.mlir b/mlir/test/Target/llvmir-invalid.mlir
index f236982d086d..dce4189f733f 100644
--- a/mlir/test/Target/llvmir-invalid.mlir
+++ b/mlir/test/Target/llvmir-invalid.mlir
@@ -12,3 +12,8 @@ llvm.func @no_nested_struct() -> !llvm<"[2 x [2 x [2 x {i32}]]]"> {
   %0 = llvm.mlir.constant(dense<[[[1, 2], [3, 4]], [[42, 43], [44, 45]]]> : tensor<2x2x2xi32>) : !llvm<"[2 x [2 x [2 x {i32}]]]">
   llvm.return %0 : !llvm<"[2 x [2 x [2 x {i32}]]]">
 }
+
+// -----
+
+// expected-error @+1 {{unsupported constant value}}
+llvm.mlir.global internal constant @test([2.5, 7.4]) : !llvm<"[2 x double]">