From 39cb2a8fc79976171b20369ff756f7fa43232b50 Mon Sep 17 00:00:00 2001 From: Alex Zinenko Date: Fri, 14 Feb 2020 10:18:19 +0100 Subject: [PATCH] [mlir] Fix argument attribute attribute reassignment in ConvertStandardToLLVM The commit switching the calling convention for memrefs (5a1778057) inadvertently introduced a bug in the function argument attribute conversion: due to incorrect indexing of function arguments it was not assigning the attributes to the arguments beyond those generated from the first original argument. This was not caught in the commit since the test suite does have a test for converting multi-argument functions with argument attributes. Fix the bug and add relevant tests. --- .../Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp | 4 ++-- mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir | 8 ++++++++ .../StandardToLLVM/convert-static-memref-ops.mlir | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp b/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp index 57ebe42ac688..f613a39a9612 100644 --- a/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp +++ b/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp @@ -921,8 +921,8 @@ protected: assert(mapping.hasValue() && "unexpected deletion of function argument"); SmallString<8> name; - for (size_t j = mapping->inputNo; j < mapping->size; ++j) { - impl::getArgAttrName(j, name); + for (size_t j = 0; j < mapping->size; ++j) { + impl::getArgAttrName(mapping->inputNo + j, name); attributes.push_back(rewriter.getNamedAttr(name, attr)); } } diff --git a/mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir b/mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir index f2cd0f8e694d..2831aefc10f9 100644 --- a/mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir +++ b/mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir @@ -9,3 +9,11 @@ func @check_attributes(%static: memref<10x20xf32> {dialect.a = true, dialect.b = 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}, %{{.*}}: !llvm{{.*}} {first.arg = true}, +// CHECK-SAME: %{{.*}}: !llvm{{.*}} {second.arg = 42 : i32}, %{{.*}}: !llvm{{.*}} {second.arg = 42 : i32}, %{{.*}}: !llvm{{.*}} {second.arg = 42 : i32}) +func @check_multiple(%first: memref {first.arg = true}, %second: memref {second.arg = 42 : i32}) { + return +} diff --git a/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir b/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir index b47d355f77f6..105a80dec7a7 100644 --- a/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir +++ b/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir @@ -3,8 +3,8 @@ // RUN: mlir-opt -convert-std-to-llvm='use-bare-ptr-memref-call-conv=1' -split-input-file %s | FileCheck %s --check-prefix=BAREPTR // BAREPTR-LABEL: func @check_noalias -// BAREPTR-SAME: %{{.*}}: !llvm<"float*"> {llvm.noalias = true} -func @check_noalias(%static : memref<2xf32> {llvm.noalias = true}) { +// BAREPTR-SAME: %{{.*}}: !llvm<"float*"> {llvm.noalias = true}, %{{.*}}: !llvm<"float*"> {llvm.noalias = true} +func @check_noalias(%static : memref<2xf32> {llvm.noalias = true}, %other : memref<2xf32> {llvm.noalias = true}) { return }