[Mlir-commits] [mlir] 551ee36 - [mlir][FuncToLLVM] Fix arg attr memref interaction

[Christian Ulmann]

Author: Christian Ulmann
Date: 2023-01-24T09:47:59+01:00
New Revision: 551ee367252a2c63bca67707c49c76c43617e7fd

URL: https://github.com/llvm/llvm-project/commit/551ee367252a2c63bca67707c49c76c43617e7fd
DIFF: https://github.com/llvm/llvm-project/commit/551ee367252a2c63bca67707c49c76c43617e7fd.diff

LOG: [mlir][FuncToLLVM] Fix arg attr memref interaction

This commit ensures that argument attributes are dropped from types
that are expanded to multiple function arguments. Previously, they
were attached to all arguments belonging to the memref, which
resulted in illegal LLVMIR, e.g., noalias on integers.

Reviewed By: gysit

Differential Revision: https://reviews.llvm.org/D142212

Added: 
    

Modified: 
    mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
    mlir/test/Conversion/FuncToLLVM/convert-argattrs.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index b18bf5658e3f0..6ff52566dee3f 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -354,9 +354,19 @@ struct FuncOpConversionBase : public ConvertOpToLLVMPattern<func::FuncOp> {
         }
         auto mapping = result.getInputMapping(i);
         assert(mapping && "unexpected deletion of function argument");
+        // Only attach the new argument attributes if there is a one-to-one
+        // mapping from old to new types. Otherwise, attributes might be
+        // attached to types that they do not support.
+        if (mapping->size == 1) {
+          newArgAttrs[mapping->inputNo] =
+              DictionaryAttr::get(rewriter.getContext(), convertedAttrs);
+          continue;
+        }
+        // TODO: Implement custom handling for types that expand to multiple
+        // function arguments.
         for (size_t j = 0; j < mapping->size; ++j)
           newArgAttrs[mapping->inputNo + j] =
-              DictionaryAttr::get(rewriter.getContext(), convertedAttrs);
+              DictionaryAttr::get(rewriter.getContext(), {});
       }
       attributes.push_back(rewriter.getNamedAttr(
           funcOp.getArgAttrsAttrName(), rewriter.getArrayAttr(newArgAttrs)));

diff  --git a/mlir/test/Conversion/FuncToLLVM/convert-argattrs.mlir b/mlir/test/Conversion/FuncToLLVM/convert-argattrs.mlir
index 0e63e628ebf8d..85c7cbddfdbf6 100644
--- a/mlir/test/Conversion/FuncToLLVM/convert-argattrs.mlir
+++ b/mlir/test/Conversion/FuncToLLVM/convert-argattrs.mlir
@@ -1,17 +1,20 @@
 // RUN: mlir-opt -convert-func-to-llvm %s | FileCheck %s
 
 // CHECK-LABEL: func @check_attributes
-// When expanding the memref to multiple arguments, argument attributes are replicated.
-// CHECK-COUNT-7: {dialect.a = true, dialect.b = 4 : i64}
-func.func @check_attributes(%static: memref<10x20xf32> {dialect.a = true, dialect.b = 4 : i64 }) {
+// CHECK-SAME: {dialect.a = true, dialect.b = 4 : i64}
+func.func @check_attributes(%int: i64 {dialect.a = true, dialect.b = 4 : i64 }) {
+  return
+}
+
+// CHECK-LABEL: func @check_memref
+// When expanding the memref to multiple arguments, argument attributes should be dropped entirely.
+// CHECK-NOT: {llvm.noalias}
+func.func @check_memref(%static: memref<10x20xf32> {llvm.noalias}) {
   return
 }
 
 // CHECK-LABEL: func @check_multiple
-// Make sure arguments attributes are attached to the right argument. We match
-// commas in the argument list for this purpose.
-// CHECK: %{{.*}}: !llvm{{.*}} {first.arg = true}, %{{.*}}: !llvm{{.*}} {first.arg = true}, %{{.*}}: i{{.*}} {first.arg = true},
-// CHECK-SAME: %{{.*}}: !llvm{{.*}} {second.arg = 42 : i32}, %{{.*}}: !llvm{{.*}} {second.arg = 42 : i32}, %{{.*}}: i{{.*}} {second.arg = 42 : i32})
-func.func @check_multiple(%first: memref<f32> {first.arg = true}, %second: memref<f32> {second.arg = 42 : i32}) {
+// CHECK-SAME: %{{.*}}: f32 {first.arg = true}, %{{.*}}: i64 {second.arg = 42 : i32}
+func.func @check_multiple(%first: f32 {first.arg = true}, %second: i64 {second.arg = 42 : i32}) {
   return
 }