From 551ee367252a2c63bca67707c49c76c43617e7fd Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann@nextsilicon.com>
Date: Tue, 24 Jan 2023 09:44:29 +0100
Subject: [PATCH] [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
---
 mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp         | 12 +++++++++++-
 mlir/test/Conversion/FuncToLLVM/convert-argattrs.mlir | 19 +++++++++++--------
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index b18bf56..6ff5256 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -354,9 +354,19 @@ protected:
         }
         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 0e63e62..85c7cbd 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
 }
-- 
2.7.4