better error reporting and nits

Author: jeanPerier

mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h

@@ -230,7 +230,8 @@ class ModuleTranslation {
 
   /// Translates parameter attributes of a call and adds them to the returned
   /// AttrBuilder. Returns failure if any of the translations failed.
-  FailureOr<llvm::AttrBuilder> convertParameterAttrs(DictionaryAttr paramAttrs);
+  FailureOr<llvm::AttrBuilder> convertParameterAttrs(CallOpInterface callOp,
+                                                     DictionaryAttr paramAttrs);
 
   /// Gets the named metadata in the LLVM IR module being constructed, creating
   /// it if it does not exist.

mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp

@@ -229,9 +229,10 @@ convertParameterAndResultAttrs(CallOpInterface callOp, llvm::CallBase *call,
                                LLVM::ModuleTranslation &moduleTranslation) {
   if (ArrayAttr argAttrsArray = callOp.getArgAttrsAttr()) {
     for (auto [argIdx, argAttrsAttr] : llvm::enumerate(argAttrsArray)) {
-      if (auto argAttrs = llvm::cast<DictionaryAttr>(argAttrsAttr)) {
+      if (auto argAttrs = cast<DictionaryAttr>(argAttrsAttr);
+          !argAttrs.empty()) {
         FailureOr<llvm::AttrBuilder> attrBuilder =
-            moduleTranslation.convertParameterAttrs(argAttrs);
+            moduleTranslation.convertParameterAttrs(callOp, argAttrs);
         if (failed(attrBuilder))
           return failure();
         call->addParamAttrs(argIdx, *attrBuilder);
@@ -240,10 +241,14 @@ convertParameterAndResultAttrs(CallOpInterface callOp, llvm::CallBase *call,
   }
 
   ArrayAttr resAttrsArray = callOp.getResAttrsAttr();
-  if (resAttrsArray && resAttrsArray.size() == 1) {
-    if (auto resAttrs = llvm::cast<DictionaryAttr>(resAttrsArray[0])) {
+  if (resAttrsArray && resAttrsArray.size() > 0) {
+    if (resAttrsArray.size() != 1)
+      return mlir::emitError(callOp.getLoc(),
+                             "llvm.func cannot have multiple results");
+    if (auto resAttrs = cast<DictionaryAttr>(resAttrsArray[0]);
+        !resAttrs.empty()) {
       FailureOr<llvm::AttrBuilder> attrBuilder =
-          moduleTranslation.convertParameterAttrs(resAttrs);
+          moduleTranslation.convertParameterAttrs(callOp, resAttrs);
       if (failed(attrBuilder))
         return failure();
       call->addRetAttrs(*attrBuilder);

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -2157,7 +2157,7 @@ void ModuleImport::convertParameterAttributes(llvm::Function *func,
 void ModuleImport::convertParameterAttributes(llvm::CallBase *call,
                                               CallOpInterface callOp,
                                               OpBuilder &builder) {
-  auto llvmAttrs = call->getAttributes();
+  llvm::AttributeList llvmAttrs = call->getAttributes();
   SmallVector<llvm::AttributeSet> llvmArgAttrsSet;
   bool anyArgAttrs = false;
   for (size_t i = 0, e = call->arg_size(); i < e; ++i) {
@@ -2181,9 +2181,8 @@ void ModuleImport::convertParameterAttributes(llvm::CallBase *call,
   llvm::AttributeSet llvmResAttr = llvmAttrs.getRetAttrs();
   if (!llvmResAttr.hasAttributes())
     return;
-  SmallVector<DictionaryAttr, 1> resAttrs;
-  resAttrs.emplace_back(convertParameterAttribute(llvmResAttr, builder));
-  callOp.setResAttrsAttr(getArrayAttr(resAttrs));
+  DictionaryAttr resAttrs = convertParameterAttribute(llvmResAttr, builder);
+  callOp.setResAttrsAttr(getArrayAttr({resAttrs}));
 }
 
 template <typename Op>

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

@@ -1563,23 +1563,33 @@ static void convertFunctionKernelAttributes(LLVMFuncOp func,
   }
 }
 
-static void convertParameterAttr(llvm::AttrBuilder &attrBuilder,
-                                 llvm::Attribute::AttrKind llvmKind,
-                                 NamedAttribute namedAttr,
-                                 ModuleTranslation &moduleTranslation) {
-  llvm::TypeSwitch<Attribute>(namedAttr.getValue())
+static LogicalResult convertParameterAttr(llvm::AttrBuilder &attrBuilder,
+                                          llvm::Attribute::AttrKind llvmKind,
+                                          NamedAttribute namedAttr,
+                                          ModuleTranslation &moduleTranslation,
+                                          Location loc) {
+  return llvm::TypeSwitch<Attribute, LogicalResult>(namedAttr.getValue())
       .Case<TypeAttr>([&](auto typeAttr) {
         attrBuilder.addTypeAttr(
             llvmKind, moduleTranslation.convertType(typeAttr.getValue()));
+        return success();
       })
       .Case<IntegerAttr>([&](auto intAttr) {
         attrBuilder.addRawIntAttr(llvmKind, intAttr.getInt());
+        return success();
+      })
+      .Case<UnitAttr>([&](auto) {
+        attrBuilder.addAttribute(llvmKind);
+        return success();
       })
-      .Case<UnitAttr>([&](auto) { attrBuilder.addAttribute(llvmKind); })
       .Case<LLVM::ConstantRangeAttr>([&](auto rangeAttr) {
         attrBuilder.addConstantRangeAttr(
             llvmKind,
             llvm::ConstantRange(rangeAttr.getLower(), rangeAttr.getUpper()));
+        return success();
+      })
+      .Default([loc](auto) {
+        return emitError(loc, "unsupported parameter attribute type");
       });
 }
 
@@ -1588,12 +1598,15 @@ ModuleTranslation::convertParameterAttrs(LLVMFuncOp func, int argIdx,
                                          DictionaryAttr paramAttrs) {
   llvm::AttrBuilder attrBuilder(llvmModule->getContext());
   auto attrNameToKindMapping = getAttrNameToKindMapping();
+  Location loc = func.getLoc();
 
   for (auto namedAttr : paramAttrs) {
     auto it = attrNameToKindMapping.find(namedAttr.getName());
     if (it != attrNameToKindMapping.end()) {
       llvm::Attribute::AttrKind llvmKind = it->second;
-      convertParameterAttr(attrBuilder, llvmKind, namedAttr, *this);
+      if (failed(convertParameterAttr(attrBuilder, llvmKind, namedAttr, *this,
+                                      loc)))
+        return failure();
     } else if (namedAttr.getNameDialect()) {
       if (failed(iface.convertParameterAttr(func, argIdx, namedAttr, *this)))
         return failure();
@@ -1604,15 +1617,19 @@ ModuleTranslation::convertParameterAttrs(LLVMFuncOp func, int argIdx,
 }
 
 FailureOr<llvm::AttrBuilder>
-ModuleTranslation::convertParameterAttrs(DictionaryAttr paramAttrs) {
+ModuleTranslation::convertParameterAttrs(CallOpInterface callOp,
+                                         DictionaryAttr paramAttrs) {
   llvm::AttrBuilder attrBuilder(llvmModule->getContext());
+  Location loc = callOp.getLoc();
   auto attrNameToKindMapping = getAttrNameToKindMapping();
 
   for (auto namedAttr : paramAttrs) {
     auto it = attrNameToKindMapping.find(namedAttr.getName());
     if (it != attrNameToKindMapping.end()) {
       llvm::Attribute::AttrKind llvmKind = it->second;
-      convertParameterAttr(attrBuilder, llvmKind, namedAttr, *this);
+      if (failed(convertParameterAttr(attrBuilder, llvmKind, namedAttr, *this,
+                                      loc)))
+        return failure();
     }
   }