[mlir] sret and byval now require a type argument when constructed

[schweitz]

Summary

Fix for change in the argument attributes in LLVM IR. This is prep work for getting FIR's code gen upstreamed.

[ftynse]

Is there a test for this?

[schweitz]

In D95378#2520616, @ftynse wrote:

Is there a test for this?

I'm not aware of tests that check the LLVM IR produced, but I didn't look super hard. Have you added them?

[mehdi_amini]

You can search in mlir/test for test that invoke mlir-translate with -mlir-to-llvmir

[ftynse]

There are tests in mlir/test/Target/llvmir.mlir. I vaguely remember asking for tests when these attributes were introduced as I'm definitely not the one who added them.

[schweitz]

In D95378#2522202, @ftynse wrote:

There are tests in mlir/test/Target/llvmir.mlir. I vaguely remember asking for tests when these attributes were introduced as I'm definitely not the one who added them.

And the tests asked for were added. (Reference: https://reviews.llvm.org/D94052)

But there was never an LLVM code gen test associated with these attributes, hence the question.

I'll look again.

[schweitz]

Add missing LLVM tests.

[ftynse]

Thanks!

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

if (auto attr = func.getArgAttrOfType<IntegerAttr>(
           llvm::AttrBuilder().addAlignmentAttr(llvm::Align(attr.getInt())));
     }
 
     if (auto attr = func.getArgAttrOfType<UnitAttr>(argIdx, "llvm.sret")) {
       auto argTy = mlirArg.getType();
       if (!argTy.isa<LLVM::LLVMPointerType>())
         return func.emitError(
             "llvm.sret attribute attached to LLVM non-pointer argument");
-      llvmArg.addAttr(llvm::Attribute::AttrKind::StructRet);
+      llvmArg.addAttrs(llvm::AttrBuilder().addStructRetAttr(
+          llvmArg.getType()->getPointerElementType()));
     }
 
     if (auto attr = func.getArgAttrOfType<UnitAttr>(argIdx, "llvm.byval")) {
       auto argTy = mlirArg.getType();
       if (!argTy.isa<LLVM::LLVMPointerType>())
         return func.emitError(
             "llvm.byval attribute attached to LLVM non-pointer argument");
-      llvmArg.addAttr(llvm::Attribute::AttrKind::ByVal);
+      llvmArg.addAttrs(llvm::AttrBuilder().addByValAttr(
+          llvmArg.getType()->getPointerElementType()));
     }
 
     valueMapping[mlirArg] = &llvmArg;
     argIdx++;
   }
 
   // Check the personality and set it.
   if (func.personality().hasValue()) {

mlir/test/Target/llvmir.mlir

// CHECK-NEXT:   br label %3
   llvm.br ^bb1(%arg1 : i1)
 }
 
 // CHECK-LABEL: define void @llvm_noalias(float* noalias {{%*.}})
 llvm.func @llvm_noalias(%arg0: !llvm.ptr<f32> {llvm.noalias = true}) {
   llvm.return
 }
 
+// CHECK-LABEL: define void @byvalattr(i32* byval(i32) %
+llvm.func @byvalattr(%arg0: !llvm.ptr<i32> {llvm.byval}) {
+  llvm.return
+}
+
+// CHECK-LABEL: define void @sretattr(i32* sret(i32) %
+llvm.func @sretattr(%arg0: !llvm.ptr<i32> {llvm.sret}) {
+  llvm.return
+}
+
 // CHECK-LABEL: define void @llvm_align(float* align 4 {{%*.}})
 llvm.func @llvm_align(%arg0: !llvm.ptr<f32> {llvm.align = 4}) {
   llvm.return
 }
 
 // CHECK-LABEL: @llvm_varargs(...)
 llvm.func @llvm_varargs(...)