[mlir][LLVMIR] Parse some type attributes for LLVM function parameters

[alexbatashev]

Summary

With the transition to opaque pointers, type information has been
transferred to function parameter attributes. This patch adds correct
parsing for some of those arguments and fixes some tests, that
previously used UnitAttr for those.

[alexbatashev]

Fix failures in Flang

[ftynse]

Thanks!

mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp

324 ↗

(On Diff #454518)

Nit: please expand auto unless the type is obvious from the line-level context.

329 ↗

(On Diff #454518)

Could we put the attr names in named constants somewhere in LLVMDialect.h? Ideally, we should also have a verifier for them in the dialect, but it may be out of scope for this patch.

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

1187

Leftover debug.

1189

Ditto.

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

849–850

Could we please have a test for user-visible error messages?

[vzakhari]

mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp

329 ↗

(On Diff #454518)

Some attributes' names are exposed via LLVMDialect::get* methods defined here.

[alexbatashev]

Address reviewers' feedback

[alexbatashev]

Thanks

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

LogicalResult Importer::processFunction(llvm::Function *f) {
   bool dsoLocal = f->hasLocalLinkage();
   CConv cconv = convertCConvFromLLVM(f->getCallingConv());
 
   b.setInsertionPoint(module.getBody(), getFuncInsertPt());
   LLVMFuncOp fop = b.create<LLVMFuncOp>(
       UnknownLoc::get(context), f->getName(), functionType,
       convertLinkageFromLLVM(f->getLinkage()), dsoLocal, cconv);
 
+  for (const auto &arg : llvm::enumerate(functionType.getParams())) {
+    llvm::SmallVector<NamedAttribute, 1> argAttrs;
+    llvm::errs() << "ARG " << arg.index() << "\n";
+    if (auto *type = f->getParamByValType(arg.index())) {
+      llvm::errs() << "BYVAL " << arg.index() << "\n";
+      auto mlirType = processType(type);
+      argAttrs.push_back(NamedAttribute(b.getStringAttr("llvm.byval"),
+                                        TypeAttr::get(mlirType)));
+    }
+    if (auto *type = f->getParamByRefType(arg.index())) {
+      auto mlirType = processType(type);
+      argAttrs.push_back(NamedAttribute(b.getStringAttr("llvm.byref"),
+                                        TypeAttr::get(mlirType)));
+    }
+    if (auto *type = f->getParamStructRetType(arg.index())) {
+      auto mlirType = processType(type);
+      argAttrs.push_back(NamedAttribute(b.getStringAttr("llvm.sret"),
+                                        TypeAttr::get(mlirType)));
+    }
+    if (auto *type = f->getParamInAllocaType(arg.index())) {
+      auto mlirType = processType(type);
+      argAttrs.push_back(NamedAttribute(b.getStringAttr("llvm.inalloca"),
+                                        TypeAttr::get(mlirType)));
+    }
+
+    fop.setArgAttrs(arg.index(), argAttrs);
+  }
+
   if (FlatSymbolRefAttr personality = getPersonalityAsAttr(f))
     fop->setAttr(b.getStringAttr("personality"), personality);
   else if (f->hasPersonalityFn())
     emitWarning(UnknownLoc::get(context),
                 "could not deduce personality, skipping it");
 
   if (f->hasGC())
     fop.setGarbageCollectorAttr(b.getStringAttr(f->getGC()));

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

if (auto attr = func.getArgAttrOfType<IntegerAttr>(
       auto argTy = mlirArg.getType();
       if (!argTy.isa<LLVM::LLVMPointerType>())
         return func.emitError(
             "llvm.align attribute attached to LLVM non-pointer argument");
       llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
                            .addAlignmentAttr(llvm::Align(attr.getInt())));
     }
 
-    if (auto attr = func.getArgAttrOfType<UnitAttr>(argIdx, "llvm.sret")) {
+    if (auto attr = func.getArgAttrOfType<TypeAttr>(argIdx, "llvm.sret")) {
       auto argTy = mlirArg.getType().dyn_cast<LLVM::LLVMPointerType>();
       if (!argTy)
         return func.emitError(
             "llvm.sret attribute attached to LLVM non-pointer argument");
-      llvmArg.addAttrs(
-          llvm::AttrBuilder(llvmArg.getContext())
-              .addStructRetAttr(convertType(argTy.getElementType())));
+      if (!argTy.isOpaque() && argTy.getElementType() != attr.getValue())
+        return func.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 = func.getArgAttrOfType<UnitAttr>(argIdx, "llvm.byval")) {
+    if (auto attr = func.getArgAttrOfType<TypeAttr>(argIdx, "llvm.byval")) {
       auto argTy = mlirArg.getType().dyn_cast<LLVM::LLVMPointerType>();
       if (!argTy)
         return func.emitError(
             "llvm.byval attribute attached to LLVM non-pointer argument");
+      if (!argTy.isOpaque() && argTy.getElementType() != attr.getValue())
+        return func.emitError("llvm.byval attribute attached to LLVM pointer "
+                              "argument of a different type");
       llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())
-                           .addByValAttr(convertType(argTy.getElementType())));
+                           .addByValAttr(convertType(attr.getValue())));
     }
 
     if (auto attr = func.getArgAttrOfType<UnitAttr>(argIdx, "llvm.nest")) {
       auto argTy = mlirArg.getType();
       if (!argTy.isa<LLVM::LLVMPointerType>())
         return func.emitError(
             "llvm.nest attribute attached to LLVM non-pointer argument");
       llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext())

mlir/test/Dialect/LLVMIR/func.mlir

attributes {foo = 42 : i32} {
     llvm.return
   }
 
   // CHECK: llvm.func @byvalattr(%{{.*}}: !llvm.ptr<i32> {llvm.byval})
   llvm.func @byvalattr(%arg0: !llvm.ptr<i32> {llvm.byval}) {
     llvm.return
   }
 
-  // CHECK: llvm.func @sretattr(%{{.*}}: !llvm.ptr<i32> {llvm.sret})
-  // LOCINFO: llvm.func @sretattr(%{{.*}}: !llvm.ptr<i32> {llvm.sret} loc("some_source_loc"))
-  llvm.func @sretattr(%arg0: !llvm.ptr<i32> {llvm.sret} loc("some_source_loc")) {
+  // 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
   }
 
   // CHECK: llvm.func @nestattr(%{{.*}}: !llvm.ptr<i32> {llvm.nest})
   llvm.func @nestattr(%arg0: !llvm.ptr<i32> {llvm.nest}) {
     llvm.return
   }
 

mlir/test/Target/LLVMIR/Import/func-attrs.ll

+; RUN: mlir-translate -import-llvm %s | FileCheck %s
+
+; CHECK: llvm.func @foo(%arg0: !llvm.ptr {llvm.byval = i64}, %arg1: !llvm.ptr {llvm.byref = i64}, %arg2: !llvm.ptr {llvm.sret = i64}, %arg3: !llvm.ptr {llvm.inalloca = i64})
+define void @foo(ptr byval(i64) %arg0, ptr byref(i64) %arg1, ptr sret(i64) %arg2, ptr inalloca(i64) %arg3) {
+  ret void
+}

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

 // expected-error @+1 {{llvm.noalias attribute attached to LLVM non-pointer argument}}
 llvm.func @invalid_noalias(%arg0 : f32 {llvm.noalias}) -> f32 {
   llvm.return %arg0 : f32
 }
 
 // -----
 
 // expected-error @+1 {{llvm.sret attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_sret(%arg0 : f32 {llvm.sret}) -> f32 {
+llvm.func @invalid_sret(%arg0 : f32 {llvm.sret = f32}) -> f32 {
   llvm.return %arg0 : f32
 }
 
 // -----
 
 // expected-error @+1 {{llvm.nest attribute attached to LLVM non-pointer argument}}
 llvm.func @invalid_nest(%arg0 : f32 {llvm.nest}) -> f32 {
   llvm.return %arg0 : f32
 }
 // -----
 
 // expected-error @+1 {{llvm.byval attribute attached to LLVM non-pointer argument}}
-llvm.func @invalid_byval(%arg0 : f32 {llvm.byval}) -> f32 {
+llvm.func @invalid_byval(%arg0 : f32 {llvm.byval = f32}) -> f32 {
   llvm.return %arg0 : f32
 }
 
 // -----
 
 // expected-error @+1 {{llvm.align attribute attached to LLVM non-pointer argument}}
 llvm.func @invalid_align(%arg0 : f32 {llvm.align = 4}) -> f32 {
   llvm.return %arg0 : f32

mlir/test/Target/LLVMIR/llvmir.mlir

 }
 
 // CHECK-LABEL: define void @llvm_noalias(ptr noalias {{%*.}})
 llvm.func @llvm_noalias(%arg0: !llvm.ptr<f32> {llvm.noalias}) {
   llvm.return
 }
 
 // CHECK-LABEL: define void @byvalattr(ptr byval(i32) %
-llvm.func @byvalattr(%arg0: !llvm.ptr<i32> {llvm.byval}) {
+llvm.func @byvalattr(%arg0: !llvm.ptr<i32> {llvm.byval = i32}) {
   llvm.return
 }
 
 // CHECK-LABEL: define void @sretattr(ptr sret(i32) %
-llvm.func @sretattr(%arg0: !llvm.ptr<i32> {llvm.sret}) {
+llvm.func @sretattr(%arg0: !llvm.ptr<i32> {llvm.sret = i32}) {
   llvm.return
 }
 
 // CHECK-LABEL: define void @nestattr(ptr nest %
 llvm.func @nestattr(%arg0: !llvm.ptr<i32> {llvm.nest}) {
   llvm.return
 }