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

[Alex Zinenko via Phabricator via llvm-commits]

ftynse created this revision.
ftynse added reviewers: rriddle, nicolasvasilache.
Herald added subscribers: llvm-commits, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, burmako, jpienaar, mehdi_amini.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75450

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


Index: mlir/test/Target/llvmir-invalid.mlir
===================================================================
--- mlir/test/Target/llvmir-invalid.mlir
+++ mlir/test/Target/llvmir-invalid.mlir
@@ -12,3 +12,8 @@
   %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]">
Index: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
===================================================================
--- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -421,7 +421,7 @@
 
 /// 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);
@@ -433,16 +433,15 @@
             llvmModule->getContext(), strAttr.getValue(), /*AddNull=*/false);
         type = cst->getType();
       } else {
-        cst = getLLVMConstant(type, op.getValueOrNull(), op.getLoc());
+        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 @@
 
     globalsMapping.try_emplace(op, var);
   }
+
+  return success();
 }
 
 /// Get the SSA value passed to the current block from the terminator operation
Index: mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
===================================================================
--- mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -56,7 +56,8 @@
       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 @@
   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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75450.247619.patch
Type: text/x-patch
Size: 3223 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200302/d90efa12/attachment.bin>