[mlir][LLVM] Add param attr verifiers

[Dinistro]

Summary

This commit introduces unified parameter attribute verifiers to the LLVM
dialect and removes according checks in the export. As LLVM does not
verify the validity of certain attributes on return values, this commit
unifies the handling of argument and result attributes wherever possible.

Depends on D142212

[gysit]

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

3136

Let's take over the original comment that explains that the operation result types may be in a transitioning state.

3182–3183

There may be downstream users for this functionality. I would keep the functionality and change the comment to "Note: The struct parameter attributes are not lowered to LLVM IR." or similar.

3210

nit: unit

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

902

This seems to be dead code?

970

nit: if it fits one line I would inline the getArgAttrDict call into the condition.

mlir/test/Dialect/LLVMIR/arg-res-attrs-invalid.mlir

109 ↗

(On Diff #491396)

nit: @variadic_def -> @void_def

113 ↗

(On Diff #491396)

Starting from here the tests seem to be duplicates?

[Dinistro]

address review comments

[Dinistro]

mlir/test/Dialect/LLVMIR/arg-res-attrs-invalid.mlir

113 ↗

(On Diff #491396)

Tests further up check argument attributes, from here on they check result attributes.

[Dinistro]

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

3182–3183

@laszlo-luminous (or @laszlokindrat) you initially pushed the struct_attr for C-wrappers. Do you know of any downstream usage of this functionality? The struct_attr functionality is not an LLVM feature and will thus be thrown away when it's exported to LLVM IR.

[definelicht]

mlir/test/Dialect/LLVMIR/arg-res-attrs-invalid.mlir

3 ↗

(On Diff #491621)

Formulation nit (same for all of these).

8 ↗

(On Diff #491621)

Formulation nit (same for all of these).

38 ↗

(On Diff #491621)

Formulation nit (same for all of these).

48 ↗

(On Diff #491621)

Formulation nit (same for all of these).

83 ↗

(On Diff #491621)

Formulation nit (same for all of these).

[Dinistro]

rename test file and add a comment to indicate the start of the respective test cases.

[Dinistro]

Address additional comments

[gysit]

LGTM!

[Dinistro]

Add checks for invalid result attributes. This checks explicitly looks for otherwise valid argument attributes, as there might be arbitrary attributes attached to a result.

[srcarroll]

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

3182–3183

@laszlokindrat is on vacation and doesn't have access, so he asked me to respond since I worked with him on this. It's been a very long time so I don't remember everything, but I remember we made these changes to fix a bug in convert-func-to-llvm=emit-c-wrappers back when the pass used to have the emit-c-wrappers option. Here's the issue I posted about it (can't remember why I'm talking about convert-std-to-llvm at first) https://github.com/llvm/llvm-project/issues/53503. Unfortunately I don't remember anything about struct_attr, but the main issue we fixed was that arg_attrs wasn't being updated correctly. So none of this was specifically intended for downstream uses, but rather a bug fix.

[Dinistro]

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

3182–3183

Thanks for the response @srcarroll. In that case the struct_attr was probably introduced with the goal of not dropping any information. Unfortunately, this is not supported by LLVM and is therefore ignored while lowering. As there seems to be no downstream usage, I will probably clean this up once this patch has landed.

can't remember why I'm talking about convert-std-to-llvm at first

The pass was originally named std-to-llvm ;)

mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td

let extraClassDeclaration = [{
     static StringRef getByValAttrName() { return "llvm.byval"; }
     static StringRef getByRefAttrName() { return "llvm.byref"; }
     static StringRef getStructRetAttrName() { return "llvm.sret"; }
     static StringRef getInAllocaAttrName() { return "llvm.inalloca"; }
     static StringRef getNoUndefAttrName() { return "llvm.noundef"; }
     static StringRef getSExtAttrName() { return "llvm.signext"; }
     static StringRef getZExtAttrName() { return "llvm.zeroext"; }
     static StringRef getTBAAAttrName() { return "llvm.tbaa"; }
+    static StringRef getNestAttrName() { return "llvm.nest"; }
 
     /// Verifies if the attribute is a well-formed value for "llvm.struct_attrs"
     static LogicalResult verifyStructAttr(
         Operation *op, Attribute attr, Type annotatedType);
 
     /// Verifies if the given string is a well-formed data layout descriptor.
     /// Uses `reportError` to report errors.
     static LogicalResult verifyDataLayoutString(
let extraClassDeclaration = [{
     static StringRef getReadnoneAttrName() {
       return "llvm.readnone";
     }
 
     Type parseType(DialectAsmParser &p) const override;
     void printType(Type, DialectAsmPrinter &p) const override;
 
   private:
+    /// Verifies a parameter attribute attached to a parameter of type
+    /// paramType.
+    LogicalResult verifyParameterAttribute(Operation *op,
+                                           Type paramType,
+                                           NamedAttribute paramAttr);
+
     /// Register all types.
     void registerTypes();
 
     /// A cache storing compatible LLVM types that have been verified. This
     /// can save us lots of verification time if there are many occurrences
     /// of some deeply-nested aggregate types in the program.
     ThreadLocalCache<DenseSet<Type>> compatibleTypes;
 

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

 // the corresponding LLVMIR module. It only handles core LLVM IR operations.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_TARGET_LLVMIR_MODULETRANSLATION_H
 #define MLIR_TARGET_LLVMIR_MODULETRANSLATION_H
 
 #include "mlir/IR/Operation.h"
-#include "mlir/IR/Value.h"
 #include "mlir/IR/SymbolTable.h"
+#include "mlir/IR/Value.h"
 #include "mlir/Target/LLVMIR/Export.h"
 #include "mlir/Target/LLVMIR/LLVMTranslationInterface.h"
 #include "mlir/Target/LLVMIR/TypeToLLVM.h"
 
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 
 namespace llvm {
explicit SaveStack(ModuleTranslation &m, Args &&...args)
       moduleTranslation.stackPush<T>(std::forward<Args>(args)...);
     }
     ~SaveStack() { moduleTranslation.stackPop(); }
 
   private:
     ModuleTranslation &moduleTranslation;
   };
 
-  SymbolTableCollection& symbolTable() { return symbolTableCollection; }
+  SymbolTableCollection &symbolTable() { return symbolTableCollection; }
 
 private:
   ModuleTranslation(Operation *module,
                     std::unique_ptr<llvm::Module> llvmModule);
   ~ModuleTranslation();
 
   /// Converts individual components.
   LogicalResult convertOperation(Operation &op, llvm::IRBuilderBase &builder);
private:
 
   /// Process tbaa LLVM Metadata operations and create LLVM
   /// metadata nodes for them.
   LogicalResult createTBAAMetadata();
 
   /// Translates dialect attributes attached to the given operation.
   LogicalResult convertDialectAttributes(Operation *op);
 
+  /// Translates parameter attributes and adds them to the returned AttrBuilder.
+  llvm::AttrBuilder convertParameterAttrs(DictionaryAttr paramAttrs);
+
   /// Original and translated module.
   Operation *mlirModule;
   std::unique_ptr<llvm::Module> llvmModule;
   /// A converter for translating debug information.
   std::unique_ptr<detail::DebugTranslation> debugTranslation;
 
   /// Builder for LLVM IR generation of OpenMP constructs.
   std::unique_ptr<llvm::OpenMPIRBuilder> ompBuilder;

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

LogicalResult LLVMDialect::verifyStructAttr(Operation *op, Attribute attr,
 
   if (structType.getBody().size() != arrAttrs.size())
     return op->emitError()
            << "size of '" << LLVMDialect::getStructAttrsAttrName()
            << "' must match the size of the annotated '!llvm.struct'";
   return success();
 }
 
-static LogicalResult verifyFuncOpInterfaceStructAttr(
-    Operation *op, Attribute attr,
-    const std::function<Type(FunctionOpInterface)> &getAnnotatedType) {
-  if (auto funcOp = dyn_cast<FunctionOpInterface>(op))
-    return LLVMDialect::verifyStructAttr(op, attr, getAnnotatedType(funcOp));
+static LogicalResult verifyFuncOpInterfaceStructAttr(Operation *op,
+                                                     Attribute attr,
+                                                     Type annotatedType) {
+  if (isa<FunctionOpInterface>(op))
+    return LLVMDialect::verifyStructAttr(op, attr, annotatedType);
   return op->emitError() << "expected '"
                          << LLVMDialect::getStructAttrsAttrName()
                          << "' to be used on function-like operations";
 }
 
-/// Verify LLVMIR function argument attributes.
-LogicalResult LLVMDialect::verifyRegionArgAttribute(Operation *op,
-                                                    unsigned regionIdx,
-                                                    unsigned argIdx,
-                                                    NamedAttribute argAttr) {
-  // Check that llvm.noalias is a unit attribute.
-  if (argAttr.getName() == LLVMDialect::getNoAliasAttrName() &&
-      !argAttr.getValue().isa<UnitAttr>())
+LogicalResult LLVMDialect::verifyParameterAttribute(Operation *op,
+                                                    Type paramType,
+                                                    NamedAttribute paramAttr) {
+  // LLVM attribute may be attached to a result of operation that has not been
+  // converted to LLVM dialect yet, so the result may have a type with unknown
+  // representation in LLVM dialect type space. In this case we cannot verify
+  // whether the attribute may be
+  bool verifyValueType = isCompatibleType(paramType);
+  StringAttr name = paramAttr.getName();
+
+  auto checkUnitAttrType = [&]() -> LogicalResult {
+    if (!paramAttr.getValue().isa<UnitAttr>())
+      return op->emitError() << name << " should be a unit attribute";
+    return success();
+  };
+  auto checkTypeAttrType = [&]() -> LogicalResult {
+    if (!paramAttr.getValue().isa<TypeAttr>())
+      return op->emitError() << name << " should be a type attribute";
+    return success();
+  };
+  auto checkIntegerAttrType = [&]() -> LogicalResult {
+    if (!paramAttr.getValue().isa<IntegerAttr>())
+      return op->emitError() << name << " should be an integer attribute";
+    return success();
+  };
+  auto checkPointerType = [&]() -> LogicalResult {
+    if (!paramType.isa<LLVMPointerType>())
       return op->emitError()
-           << "expected llvm.noalias argument attribute to be a unit attribute";
-  // Check that llvm.align is an integer attribute.
-  if (argAttr.getName() == LLVMDialect::getAlignAttrName() &&
-      !argAttr.getValue().isa<IntegerAttr>())
+             << name << " attribute attached to non-pointer LLVM type";
+    return success();
+  };
+  auto checkIntegerType = [&]() -> LogicalResult {
+    if (!paramType.isa<IntegerType>())
       return op->emitError()
-           << "llvm.align argument attribute of non integer type";
-  if (argAttr.getName() == LLVMDialect::getStructAttrsAttrName()) {
-    return verifyFuncOpInterfaceStructAttr(
-        op, argAttr.getValue(), [argIdx](FunctionOpInterface funcOp) {
-          return funcOp.getArgumentTypes()[argIdx];
-        });
-  }
+             << name << " attribute attached to non-integer LLVM type";
     return success();
-}
-
-LogicalResult LLVMDialect::verifyRegionResultAttribute(Operation *op,
-                                                       unsigned regionIdx,
-                                                       unsigned resIdx,
-                                                       NamedAttribute resAttr) {
-  StringAttr name = resAttr.getName();
-  if (name == LLVMDialect::getStructAttrsAttrName()) {
-    return verifyFuncOpInterfaceStructAttr(
-        op, resAttr.getValue(), [resIdx](FunctionOpInterface funcOp) {
-          return funcOp.getResultTypes()[resIdx];
-        });
-  }
-  if (auto funcOp = dyn_cast<FunctionOpInterface>(op)) {
-    mlir::Type resTy = funcOp.getResultTypes()[resIdx];
+  };
+  auto checkPointerTypeMatches = [&]() -> LogicalResult {
+    if (failed(checkPointerType()))
+      return failure();
+    auto ptrType = paramType.cast<LLVMPointerType>();
+    auto typeAttr = paramAttr.getValue().cast<TypeAttr>();
 
-    // Check to see if this function has a void return with a result attribute
-    // to it. It isn't clear what semantics we would assign to that.
-    if (resTy.isa<LLVMVoidType>())
-      return op->emitError() << "cannot attach result attributes to functions "
-                                "with a void return";
+    if (!ptrType.isOpaque() && ptrType.getElementType() != typeAttr.getValue())
+      return op->emitError()
+             << name
+             << " attribute attached to LLVM pointer argument of "
+                "different type";
+    return success();
+  };
 
-    // LLVM attribute may be attached to a result of operation
-    // that has not been converted to LLVM dialect yet, so the result
-    // may have a type with unknown representation in LLVM dialect type
-    // space. In this case we cannot verify whether the attribute may be
-    // attached to a result of such type.
-    bool verifyValueType = isCompatibleType(resTy);
-    Attribute attrValue = resAttr.getValue();
+  // Note: The struct parameter attributes are not lowered to LLVM IR.
+  if (name == LLVMDialect::getStructAttrsAttrName())
+    return verifyFuncOpInterfaceStructAttr(op, paramAttr.getValue(), paramType);
 
-    // TODO: get rid of code duplication here and in verifyRegionArgAttribute().
-    if (name == LLVMDialect::getAlignAttrName()) {
-      if (!attrValue.isa<IntegerAttr>())
-        return op->emitError() << "expected llvm.align result attribute to be "
-                                  "an integer attribute";
-      if (verifyValueType && !resTy.isa<LLVMPointerType>())
-        return op->emitError()
-               << "llvm.align attribute attached to non-pointer result";
-      return success();
-    }
-    if (name == LLVMDialect::getNoAliasAttrName()) {
-      if (!attrValue.isa<UnitAttr>())
-        return op->emitError() << "expected llvm.noalias result attribute to "
-                                  "be a unit attribute";
-      if (verifyValueType && !resTy.isa<LLVMPointerType>())
-        return op->emitError()
-               << "llvm.noalias attribute attached to non-pointer result";
-      return success();
-    }
-    if (name == LLVMDialect::getReadonlyAttrName()) {
-      if (!attrValue.isa<UnitAttr>())
-        return op->emitError() << "expected llvm.readonly result attribute to "
-                                  "be a unit attribute";
-      if (verifyValueType && !resTy.isa<LLVMPointerType>())
-        return op->emitError()
-               << "llvm.readonly attribute attached to non-pointer result";
-      return success();
-    }
-    if (name == LLVMDialect::getNoUndefAttrName()) {
-      if (!attrValue.isa<UnitAttr>())
-        return op->emitError() << "expected llvm.noundef result attribute to "
-                                  "be a unit attribute";
-      return success();
-    }
-    if (name == LLVMDialect::getSExtAttrName()) {
-      if (!attrValue.isa<UnitAttr>())
-        return op->emitError() << "expected llvm.signext result attribute to "
-                                  "be a unit attribute";
-      if (verifyValueType && !resTy.isa<mlir::IntegerType>())
-        return op->emitError()
-               << "llvm.signext attribute attached to non-integer result";
-      return success();
-    }
-    if (name == LLVMDialect::getZExtAttrName()) {
-      if (!attrValue.isa<UnitAttr>())
-        return op->emitError() << "expected llvm.zeroext result attribute to "
-                                  "be a unit attribute";
-      if (verifyValueType && !resTy.isa<mlir::IntegerType>())
-        return op->emitError()
-               << "llvm.zeroext attribute attached to non-integer result";
-      return success();
-    }
-  }
+  // Check a unit attribute that is attached to a pointer value.
+  if (name == LLVMDialect::getNoAliasAttrName() ||
+      name == LLVMDialect::getReadonlyAttrName() ||
+      name == LLVMDialect::getNestAttrName()) {
+    if (failed(checkUnitAttrType()))
+      return failure();
+    if (verifyValueType && failed(checkPointerType()))
+      return failure();
+
+  // Check a type attribute that is attached to a pointer value.
+  if (name == LLVMDialect::getStructRetAttrName() ||
+      name == LLVMDialect::getByValAttrName() ||
+      name == LLVMDialect::getByRefAttrName() ||
+      name == LLVMDialect::getInAllocaAttrName()) {
+    if (failed(checkTypeAttrType()))
+      return failure();
+    if (verifyValueType && failed(checkPointerTypeMatches()))
+      return failure();
+
+  // Check a unit attribute that is attached to an integer value.
+  if (name == LLVMDialect::getSExtAttrName() ||
+      name == LLVMDialect::getZExtAttrName()) {
+    if (failed(checkUnitAttrType()))
+      return failure();
+    if (verifyValueType && failed(checkIntegerType()))
+      return failure();
+
+  // Check an integer attribute that is attached to a pointer value.
+  if (name == LLVMDialect::getAlignAttrName()) {
+    if (failed(checkIntegerAttrType()))
+      return failure();
+    if (verifyValueType && failed(checkPointerType()))
+      return failure();
+
+  if (name == LLVMDialect::getNoUndefAttrName())
+    return checkUnitAttrType();
+
+/// Verify LLVMIR function argument attributes.
+LogicalResult LLVMDialect::verifyRegionArgAttribute(Operation *op,
+                                                    unsigned regionIdx,
+                                                    unsigned argIdx,
+                                                    NamedAttribute argAttr) {
+  auto funcOp = dyn_cast<FunctionOpInterface>(op);
+  if (!funcOp)
+  Type argType = funcOp.getArgumentTypes()[argIdx];
+
+  return verifyParameterAttribute(op, argType, argAttr);
 
+LogicalResult LLVMDialect::verifyRegionResultAttribute(Operation *op,
+                                                       unsigned regionIdx,
+                                                       unsigned resIdx,
+                                                       NamedAttribute resAttr) {
+  auto funcOp = dyn_cast<FunctionOpInterface>(op);
+  if (!funcOp)
     return success();
+  Type resType = funcOp.getResultTypes()[resIdx];
+
+  // Check to see if this function has a void return with a result attribute
+  // to it. It isn't clear what semantics we would assign to that.
+  if (resType.isa<LLVMVoidType>())
+    return op->emitError() << "cannot attach result attributes to functions "
+                              "with a void return";
+
+  return verifyParameterAttribute(op, resType, resAttr);
 }
 
 //===----------------------------------------------------------------------===//
 // Utility functions.
 //===----------------------------------------------------------------------===//
 
 Value mlir::LLVM::createGlobalString(Location loc, OpBuilder &builder,
                                      StringRef name, StringRef value,

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

newMemEffects |= llvm::MemoryEffects(
       llvm::MemoryEffects::Location::InaccessibleMem,
       convertModRefInfoToLLVM(memEffects.getInaccessibleMem()));
   newMemEffects |=
       llvm::MemoryEffects(llvm::MemoryEffects::Location::Other,
                           convertModRefInfoToLLVM(memEffects.getOther()));
   llvmFunc->setMemoryEffects(newMemEffects);
 }
 
+llvm::AttrBuilder
+ModuleTranslation::convertParameterAttrs(DictionaryAttr paramAttrs) {
+  llvm::AttrBuilder attrBuilder(llvmModule->getContext());
+  if (auto attr = paramAttrs.getAs<UnitAttr>(LLVMDialect::getNoAliasAttrName()))
+    attrBuilder.addAttribute(llvm::Attribute::AttrKind::NoAlias);
+
+  if (auto attr =
+          paramAttrs.getAs<UnitAttr>(LLVMDialect::getReadonlyAttrName()))
+    attrBuilder.addAttribute(llvm::Attribute::AttrKind::ReadOnly);
+
+  if (auto attr =
+          paramAttrs.getAs<IntegerAttr>(LLVMDialect::getAlignAttrName()))
+    attrBuilder.addAlignmentAttr(llvm::Align(attr.getInt()));
+
+  if (auto attr =
+          paramAttrs.getAs<TypeAttr>(LLVMDialect::getStructRetAttrName()))
+    attrBuilder.addStructRetAttr(convertType(attr.getValue()));
+
+  if (auto attr = paramAttrs.getAs<TypeAttr>(LLVMDialect::getByValAttrName()))
+    attrBuilder.addByValAttr(convertType(attr.getValue()));
+
+  if (auto attr = paramAttrs.getAs<TypeAttr>(LLVMDialect::getByRefAttrName()))
+    attrBuilder.addByRefAttr(convertType(attr.getValue()));
+
+  if (auto attr =
+          paramAttrs.getAs<TypeAttr>(LLVMDialect::getInAllocaAttrName()))
+    attrBuilder.addInAllocaAttr(convertType(attr.getValue()));
+
+  if (auto attr = paramAttrs.getAs<UnitAttr>(LLVMDialect::getNestAttrName()))
+    attrBuilder.addAttribute(llvm::Attribute::Nest);
+
+  if (auto attr = paramAttrs.getAs<UnitAttr>(LLVMDialect::getNoUndefAttrName()))
+    attrBuilder.addAttribute(llvm::Attribute::NoUndef);
+
+  if (auto attr = paramAttrs.getAs<UnitAttr>(LLVMDialect::getSExtAttrName()))
+    attrBuilder.addAttribute(llvm::Attribute::SExt);
+
+  if (auto attr = paramAttrs.getAs<UnitAttr>(LLVMDialect::getZExtAttrName()))
+    attrBuilder.addAttribute(llvm::Attribute::ZExt);
+  return attrBuilder;
+}
+
 LogicalResult ModuleTranslation::convertFunctionSignatures() {
   // Declare all functions first because there may be function calls that form a
   // call graph with cycles, or global initializers that reference functions.
   for (auto function : getModuleBody(mlirModule).getOps<LLVMFuncOp>()) {
     llvm::FunctionCallee llvmFuncCst = llvmModule->getOrInsertFunction(
         function.getName(),
         cast<llvm::FunctionType>(convertType(function.getFunctionType())));
     llvm::Function *llvmFunc = cast<llvm::Function>(llvmFuncCst.getCallee());
     llvmFunc->setLinkage(convertLinkageToLLVM(function.getLinkage()));
     llvmFunc->setCallingConv(convertCConvToLLVM(function.getCConv()));
     mapFunction(function.getName(), llvmFunc);
     addRuntimePreemptionSpecifier(function.getDsoLocal(), llvmFunc);
 
     // Convert function attributes.
     convertFunctionAttributes(function, llvmFunc);
 
     // Convert function_entry_count attribute to metadata.
     if (std::optional<uint64_t> entryCount = function.getFunctionEntryCount())
       llvmFunc->setEntryCount(entryCount.value());
 
     // Convert result attributes.
     if (ArrayAttr allResultAttrs = function.getAllResultAttrs()) {
-      llvm::AttrBuilder retAttrs(llvmFunc->getContext());
       DictionaryAttr resultAttrs = allResultAttrs[0].cast<DictionaryAttr>();
-      for (const NamedAttribute &attr : resultAttrs) {
-        StringAttr name = attr.getName();
-        if (name == LLVMDialect::getAlignAttrName()) {
-          auto alignAmount = attr.getValue().cast<IntegerAttr>();
-          retAttrs.addAlignmentAttr(llvm::Align(alignAmount.getInt()));
-        } else if (name == LLVMDialect::getNoAliasAttrName()) {
-          retAttrs.addAttribute(llvm::Attribute::NoAlias);
-        } else if (name == LLVMDialect::getNoUndefAttrName()) {
-          retAttrs.addAttribute(llvm::Attribute::NoUndef);
-        } else if (name == LLVMDialect::getSExtAttrName()) {
-          retAttrs.addAttribute(llvm::Attribute::SExt);
-        } else if (name == LLVMDialect::getZExtAttrName()) {
-          retAttrs.addAttribute(llvm::Attribute::ZExt);
-        }
-      }
-      llvmFunc->addRetAttrs(retAttrs);
+      llvmFunc->addRetAttrs(convertParameterAttrs(resultAttrs));
     }
 
     // Convert argument attributes.
-    unsigned int argIdx = 0;
-    for (auto [mlirArgTy, llvmArg] :
-         llvm::zip(function.getArgumentTypes(), llvmFunc->args())) {
-      if (auto attr = function.getArgAttrOfType<UnitAttr>(
-              argIdx, LLVMDialect::getNoAliasAttrName())) {
-        // NB: Attribute already verified to be boolean, so check if we can
-        // indeed attach the attribute to this argument, based on its type.
-        if (!mlirArgTy.isa<LLVM::LLVMPointerType>())
-          return function.emitError(
-              "llvm.noalias attribute attached to LLVM non-pointer argument");
-        llvmArg.addAttr(llvm::Attribute::AttrKind::NoAlias);
-      }
-      if (auto attr = function.getArgAttrOfType<UnitAttr>(
-              argIdx, LLVMDialect::getReadonlyAttrName())) {
-        if (!mlirArgTy.isa<LLVM::LLVMPointerType>())
-          return function.emitError(
-              "llvm.readonly attribute attached to LLVM non-pointer argument");
-        llvmArg.addAttr(llvm::Attribute::AttrKind::ReadOnly);
-      }
-
-      if (auto attr = function.getArgAttrOfType<IntegerAttr>(
-              argIdx, LLVMDialect::getAlignAttrName())) {
-        // NB: Attribute already verified to be int, so check if we can indeed
-        // attach the attribute to this argument, based on its type.
-        if (!mlirArgTy.isa<LLVM::LLVMPointerType>())
-          return function.emitError(
-              "llvm.align attribute attached to LLVM non-pointer argument");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addAlignmentAttr(llvm::Align(attr.getInt())));
-      }
-
-      if (auto attr = function.getArgAttrOfType<TypeAttr>(
-              argIdx, LLVMDialect::getStructRetAttrName())) {
-        auto argTy = mlirArgTy.dyn_cast<LLVM::LLVMPointerType>();
-        if (!argTy)
-          return function.emitError(
-              "llvm.sret attribute attached to LLVM non-pointer argument");
-        if (!argTy.isOpaque() && argTy.getElementType() != attr.getValue())
-          return function.emitError(
-              "llvm.sret attribute attached to LLVM pointer "
-              "argument of a different type");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addStructRetAttr(convertType(attr.getValue())));
-      }
-
-      if (auto attr = function.getArgAttrOfType<TypeAttr>(
-              argIdx, LLVMDialect::getByValAttrName())) {
-        auto argTy = mlirArgTy.dyn_cast<LLVM::LLVMPointerType>();
-        if (!argTy)
-          return function.emitError(
-              "llvm.byval attribute attached to LLVM non-pointer argument");
-        if (!argTy.isOpaque() && argTy.getElementType() != attr.getValue())
-          return function.emitError(
-              "llvm.byval attribute attached to LLVM pointer "
-              "argument of a different type");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addByValAttr(convertType(attr.getValue())));
-      }
-
-      if (auto attr = function.getArgAttrOfType<TypeAttr>(
-              argIdx, LLVMDialect::getByRefAttrName())) {
-        auto argTy = mlirArgTy.dyn_cast<LLVM::LLVMPointerType>();
-        if (!argTy)
-          return function.emitError(
-              "llvm.byref attribute attached to LLVM non-pointer argument");
-        if (!argTy.isOpaque() && argTy.getElementType() != attr.getValue())
-          return function.emitError(
-              "llvm.byref attribute attached to LLVM pointer "
-              "argument of a different type");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addByRefAttr(convertType(attr.getValue())));
-      }
-
-      if (auto attr = function.getArgAttrOfType<TypeAttr>(
-              argIdx, LLVMDialect::getInAllocaAttrName())) {
-        auto argTy = mlirArgTy.dyn_cast<LLVM::LLVMPointerType>();
-        if (!argTy)
-          return function.emitError(
-              "llvm.inalloca attribute attached to LLVM non-pointer argument");
-        if (!argTy.isOpaque() && argTy.getElementType() != attr.getValue())
-          return function.emitError(
-              "llvm.inalloca attribute attached to LLVM pointer "
-              "argument of a different type");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addInAllocaAttr(convertType(attr.getValue())));
+    for (auto [argIdx, llvmArg] : llvm::enumerate(llvmFunc->args())) {
+      if (DictionaryAttr argAttrs = function.getArgAttrDict(argIdx)) {
+        llvm::AttrBuilder attrBuilder = convertParameterAttrs(argAttrs);
+        llvmArg.addAttrs(attrBuilder);
       }
-
-      if (auto attr =
-              function.getArgAttrOfType<UnitAttr>(argIdx, "llvm.nest")) {
-        if (!mlirArgTy.isa<LLVM::LLVMPointerType>())
-          return function.emitError(
-              "llvm.nest attribute attached to LLVM non-pointer argument");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addAttribute(llvm::Attribute::Nest));
-      }
-
-      if (auto attr = function.getArgAttrOfType<UnitAttr>(
-              argIdx, LLVMDialect::getNoUndefAttrName())) {
-        // llvm.noundef can be added to any argument type.
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addAttribute(llvm::Attribute::NoUndef));
-      }
-      if (auto attr = function.getArgAttrOfType<UnitAttr>(
-              argIdx, LLVMDialect::getSExtAttrName())) {
-        // llvm.signext can be added to any integer argument type.
-        if (!mlirArgTy.isa<mlir::IntegerType>())
-          return function.emitError(
-              "llvm.signext attribute attached to LLVM non-integer argument");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addAttribute(llvm::Attribute::SExt));
-      }
-      if (auto attr = function.getArgAttrOfType<UnitAttr>(
-              argIdx, LLVMDialect::getZExtAttrName())) {
-        // llvm.zeroext can be added to any integer argument type.
-        if (!mlirArgTy.isa<mlir::IntegerType>())
-          return function.emitError(
-              "llvm.zeroext attribute attached to LLVM non-integer argument");
-        llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                             .addAttribute(llvm::Attribute::ZExt));
-      }
-
-      ++argIdx;
     }
 
     // Forward the pass-through attributes to LLVM.
     if (failed(forwardPassthroughAttributes(
             function.getLoc(), function.getPassthrough(), llvmFunc)))
       return failure();
   }
 

mlir/test/Dialect/LLVMIR/func.mlir

module {
 
   // CHECK: llvm.func @roundtrip12(%{{.*}}: !llvm.ptr<i32> {llvm.noalias})
   // CHECK: attributes {foo = 42 : i32}
   llvm.func @roundtrip12(%arg0: !llvm.ptr<i32> {llvm.noalias})
   attributes {foo = 42 : i32} {
     llvm.return
   }
 
-  // CHECK: llvm.func @byvalattr(%{{.*}}: !llvm.ptr<i32> {llvm.byval})
-  llvm.func @byvalattr(%arg0: !llvm.ptr<i32> {llvm.byval}) {
+  // CHECK: llvm.func @byvalattr(%{{.*}}: !llvm.ptr<i32> {llvm.byval = i32})
+  llvm.func @byvalattr(%arg0: !llvm.ptr<i32> {llvm.byval = i32}) {
     llvm.return
   }
 
   // CHECK: llvm.func @sretattr(%{{.*}}: !llvm.ptr<i32> {llvm.sret = i32})
   // LOCINFO: llvm.func @sretattr(%{{.*}}: !llvm.ptr<i32> {llvm.sret = i32} loc("some_source_loc"))
   llvm.func @sretattr(%arg0: !llvm.ptr<i32> {llvm.sret = i32} loc("some_source_loc")) {
     llvm.return
   }

 module {
   // expected-error@+1 {{failed to construct function type: expected zero or one function result}}
   llvm.func @foo() -> (i64, i64)
 }
 
 // -----
 
 module {
-  // expected-error@+1 {{cannot attach result attributes to functions with a void return}}
-  llvm.func @variadic_def() -> (!llvm.void {llvm.noundef})
-}
-
-// -----
-
-// expected-error @below{{expected llvm.align result attribute to be an integer attribute}}
-llvm.func @alignattr_ret() -> (!llvm.ptr {llvm.align = 1.0 : f32})
-
-// -----
-
-// expected-error @below{{llvm.align attribute attached to non-pointer result}}
-llvm.func @alignattr_ret() -> (i32 {llvm.align = 4})
-
-// -----
-
-// expected-error @below{{expected llvm.noalias result attribute to be a unit attribute}}
-llvm.func @noaliasattr_ret() -> (!llvm.ptr {llvm.noalias = 1})
-
-// -----
-
-// expected-error @below{{llvm.noalias attribute attached to non-pointer result}}
-llvm.func @noaliasattr_ret() -> (i32 {llvm.noalias})
-
-// -----
-
-// expected-error @below{{expected llvm.noundef result attribute to be a unit attribute}}
-llvm.func @noundefattr_ret() -> (!llvm.ptr {llvm.noundef = 1})
-
-// -----
-
-// expected-error @below{{expected llvm.signext result attribute to be a unit attribute}}
-llvm.func @signextattr_ret() -> (i32 {llvm.signext = 1})
-
-// -----
-
-// expected-error @below{{llvm.signext attribute attached to non-integer result}}
-llvm.func @signextattr_ret() -> (f32 {llvm.signext})
-
-// -----
-
-// expected-error @below{{expected llvm.zeroext result attribute to be a unit attribute}}
-llvm.func @zeroextattr_ret() -> (i32 {llvm.zeroext = 1})
-
-// -----
-
-// expected-error @below{{llvm.zeroext attribute attached to non-integer result}}
-llvm.func @zeroextattr_ret() -> (f32 {llvm.zeroext})
-
-// -----
-
-module {
   // expected-error@+1 {{variadic arguments must be in the end of the argument list}}
   llvm.func @variadic_inside(%arg0: i32, ..., %arg1: i32)
 }
 
 // -----
 
 module {
   // expected-error@+1 {{external functions must have 'external' or 'extern_weak' linkage}}

mlir/test/Dialect/LLVMIR/invalid.mlir

 
 // -----
 
 llvm.func @dtor()
 
 // expected-error@+1{{'dtor' does not have a definition}}
 llvm.mlir.global_dtors {dtors = [@dtor], priorities = [0 : i32]}
 
-// -----
-
-// expected-error@+1{{expected llvm.noalias argument attribute to be a unit attribute}}
-func.func @invalid_noalias(%arg0: i32 {llvm.noalias = 3}) {
-  "llvm.return"() : () -> ()
-}
-
-// -----
-
-// expected-error@+1{{llvm.align argument attribute of non integer type}}
-func.func @invalid_align(%arg0: i32 {llvm.align = "foo"}) {
-  "llvm.return"() : () -> ()
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
 // Check that parser errors are properly produced and do not crash the compiler.
 
 // -----
 
 func.func @icmp_non_string(%arg0 : i32, %arg1 : i16) {
   // expected-error@+1 {{invalid kind of attribute specified}}

mlir/test/Dialect/LLVMIR/parameter-attrs-invalid.mlir

+// RUN: mlir-opt %s -split-input-file -verify-diagnostics
+
+// Argument attributes
+
+// expected-error@below {{"llvm.noalias" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_noalias_arg_type(%0 : i32 {llvm.noalias})
+
+// -----
+
+// expected-error@below {{"llvm.noalias" should be a unit attribute}}
+llvm.func @invalid_noalias_attr_type(%0 : !llvm.ptr {llvm.noalias = 10 : i32})
+
+// -----
+
+// expected-error@below {{"llvm.readonly" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_readonly_arg_type(%0 : i32 {llvm.readonly})
+
+// -----
+
+// expected-error@below {{"llvm.readonly" should be a unit attribute}}
+llvm.func @invalid_readonly_attr_type(%0 : i32 {llvm.readonly = i32})
+
+// -----
+
+// expected-error@below {{"llvm.nest" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_nest_arg_type(%0 : i32 {llvm.nest})
+
+// -----
+
+// expected-error@below {{"llvm.nest" should be a unit attribute}}
+llvm.func @invalid_nest_attr_type(%0 : i32 {llvm.nest = "foo"})
+
+// -----
+
+// expected-error@below {{"llvm.align" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_align_arg_type(%0 : i32 {llvm.align = 10 : i32})
+
+// -----
+
+// expected-error@below {{"llvm.align" should be an integer attribute}}
+llvm.func @invalid_align_attr_type(%0 : i32 {llvm.align = "foo"})
+
+// -----
+
+// expected-error@below {{"llvm.sret" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_sret_arg_type(%0 : i32 {llvm.sret = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.sret" attribute attached to LLVM pointer argument of different type}}
+llvm.func @invalid_sret_attr_type(%0 : !llvm.ptr<f32> {llvm.sret = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.byval" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_byval_arg_type(%0 : i32 {llvm.byval = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.byval" attribute attached to LLVM pointer argument of different type}}
+llvm.func @invalid_byval_attr_type(%0 : !llvm.ptr<!llvm.struct<(f32)>> {llvm.byval = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.byref" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_byref_arg_type(%0 : i32 {llvm.byref = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.byref" attribute attached to LLVM pointer argument of different type}}
+llvm.func @invalid_byref_attr_type(%0 : !llvm.ptr<!llvm.struct<(f32)>> {llvm.byref = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.inalloca" attribute attached to non-pointer LLVM type}}
+llvm.func @invalid_inalloca_arg_type(%0 : i32 {llvm.inalloca = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.inalloca" attribute attached to LLVM pointer argument of different type}}
+llvm.func @invalid_inalloca_attr_type(%0 : !llvm.ptr<!llvm.struct<(f32)>> {llvm.inalloca = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.signext" attribute attached to non-integer LLVM type}}
+llvm.func @invalid_signext_arg_type(%0 : f32 {llvm.signext})
+
+// -----
+
+// expected-error@below {{"llvm.signext" should be a unit attribute}}
+llvm.func @invalid_signext_attr_type(%0 : i32 {llvm.signext = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.zeroext" attribute attached to non-integer LLVM type}}
+llvm.func @invalid_zeroext_arg_type(%0 : f32 {llvm.zeroext})
+
+// -----
+
+// expected-error@below {{"llvm.zeroext" should be a unit attribute}}
+llvm.func @invalid_zeroext_attr_type(%0 : i32 {llvm.zeroext = !llvm.struct<(i32)>})
+
+// -----
+
+// expected-error@below {{"llvm.noundef" should be a unit attribute}}
+llvm.func @invalid_noundef_attr_type(%0 : i32 {llvm.noundef = !llvm.ptr})
+
+// -----
+
+// Result attributes
+
+// expected-error@below {{cannot attach result attributes to functions with a void return}}
+llvm.func @void_def() -> (!llvm.void {llvm.noundef})
+
+// -----
+
+// expected-error @below{{"llvm.align" should be an integer attribute}}
+llvm.func @alignattr_ret() -> (!llvm.ptr {llvm.align = 1.0 : f32})
+
+// -----
+
+// expected-error @below{{"llvm.align" attribute attached to non-pointer LLVM type}}
+llvm.func @alignattr_ret() -> (i32 {llvm.align = 4})
+
+// -----
+
+// expected-error @below{{"llvm.noalias" should be a unit attribute}}
+llvm.func @noaliasattr_ret() -> (!llvm.ptr {llvm.noalias = 1})
+
+// -----
+
+// expected-error @below{{"llvm.noalias" attribute attached to non-pointer LLVM type}}
+llvm.func @noaliasattr_ret() -> (i32 {llvm.noalias})
+
+// -----
+
+// expected-error @below{{"llvm.noundef" should be a unit attribute}}
+llvm.func @noundefattr_ret() -> (!llvm.ptr {llvm.noundef = 1})
+
+// -----
+
+// expected-error @below{{"llvm.signext" should be a unit attribute}}
+llvm.func @signextattr_ret() -> (i32 {llvm.signext = 1})
+
+// -----
+
+// expected-error @below{{"llvm.signext" attribute attached to non-integer LLVM type}}
+llvm.func @signextattr_ret() -> (f32 {llvm.signext})
+
+// -----
+
+// expected-error @below{{"llvm.zeroext" should be a unit attribute}}
+llvm.func @zeroextattr_ret() -> (i32 {llvm.zeroext = 1})
+
+// -----
+
+// expected-error @below{{"llvm.zeroext" attribute attached to non-integer LLVM type}}
+llvm.func @zeroextattr_ret() -> (f32 {llvm.zeroext})

mlir/test/Target/LLVMIR/llvmir-invalid.mlir

 // RUN: mlir-translate -verify-diagnostics -split-input-file -mlir-to-llvmir %s
 
 // expected-error @below{{cannot be converted to LLVM IR}}
 func.func @foo() {
   llvm.return
 }
 
 // -----
 
-// expected-error @below{{llvm.noalias attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_noalias(%arg0 : f32 {llvm.noalias}) -> f32 {
-  llvm.return %arg0 : f32
-}
-
-// -----
-
-// expected-error @below{{llvm.sret attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_sret(%arg0 : f32 {llvm.sret = f32}) -> f32 {
-  llvm.return %arg0 : f32
-}
-
-// -----
-
-// expected-error @below{{llvm.sret attribute attached to LLVM pointer argument of a different type}}
-llvm.func @invalid_sret(%arg0 : !llvm.ptr<f32> {llvm.sret = i32}) -> !llvm.ptr<f32> {
-  llvm.return %arg0 : !llvm.ptr<f32>
-}
-
-// -----
-
-// expected-error @below{{llvm.nest attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_nest(%arg0 : f32 {llvm.nest}) -> f32 {
-  llvm.return %arg0 : f32
-}
-// -----
-
-// expected-error @below{{llvm.byval attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_byval(%arg0 : f32 {llvm.byval = f32}) -> f32 {
-  llvm.return %arg0 : f32
-}
-
-// -----
-
-// expected-error @below{{llvm.byval attribute attached to LLVM pointer argument of a different type}}
-llvm.func @invalid_sret(%arg0 : !llvm.ptr<f32> {llvm.byval = i32}) -> !llvm.ptr<f32> {
-  llvm.return %arg0 : !llvm.ptr<f32>
-}
-
-// -----
-
-// expected-error @below{{llvm.byref attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_byval(%arg0 : f32 {llvm.byref = f32}) -> f32 {
-  llvm.return %arg0 : f32
-}
-
-// -----
-
-// expected-error @below{{llvm.byref attribute attached to LLVM pointer argument of a different type}}
-llvm.func @invalid_sret(%arg0 : !llvm.ptr<f32> {llvm.byref = i32}) -> !llvm.ptr<f32> {
-  llvm.return %arg0 : !llvm.ptr<f32>
-}
-
-// -----
-
-// expected-error @below{{llvm.inalloca attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_byval(%arg0 : f32 {llvm.inalloca = f32}) -> f32 {
-  llvm.return %arg0 : f32
-}
-
-// -----
-
-// expected-error @below{{llvm.inalloca attribute attached to LLVM pointer argument of a different type}}
-llvm.func @invalid_sret(%arg0 : !llvm.ptr<f32> {llvm.inalloca = i32}) -> !llvm.ptr<f32> {
-  llvm.return %arg0 : !llvm.ptr<f32>
-}
-
-// -----
-
-// expected-error @below{{llvm.align attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_align(%arg0 : f32 {llvm.align = 4}) -> f32 {
-  llvm.return %arg0 : f32
-}
-
-// -----
-
-// expected-error @below{{llvm.signext attribute attached to LLVM non-integer argument}}
-llvm.func @invalid_signext(%arg0: f32 {llvm.signext}) {
-  "llvm.return"() : () -> ()
-}
-
-// -----
-
-// expected-error @below{{llvm.zeroext attribute attached to LLVM non-integer argument}}
-llvm.func @invalid_zeroext(%arg0: f32 {llvm.zeroext}) {
-  "llvm.return"() : () -> ()
-}
-
-// -----
-
 llvm.func @no_non_complex_struct() -> !llvm.array<2 x array<2 x array<2 x struct<(i32)>>>> {
   // expected-error @below{{expected struct type to be a complex number}}
   %0 = llvm.mlir.constant(dense<[[[1, 2], [3, 4]], [[42, 43], [44, 45]]]> : tensor<2x2x2xi32>) : !llvm.array<2 x array<2 x array<2 x struct<(i32)>>>>
   llvm.return %0 : !llvm.array<2 x array<2 x array<2 x struct<(i32)>>>>
 }
 
 // -----
 

 
 // -----
 
 llvm.func @stepvector_intr_wrong_type() -> vector<7xf32> {
   // expected-error @below{{op result #0 must be LLVM dialect-compatible vector of signless integer, but got 'vector<7xf32>'}}
   %0 = llvm.intr.experimental.stepvector : vector<7xf32>
   llvm.return %0 : vector<7xf32>
 }
-
-// -----
-
-// expected-error @below{{llvm.readonly attribute attached to LLVM non-pointer argument}}
-llvm.func @wrong_readonly_attribute(%vec : f32 {llvm.readonly}) {
-  llvm.return
-}