[PATCH] D74211: [mlir] use unpacked memref descriptors at function boundaries

[Alex Zinenko via Phabricator via llvm-commits]

ftynse added a comment.

In D74211#1877293 <https://reviews.llvm.org/D74211#1877293>, @dcaballe wrote:

> @ayzhuang found another case that is breaking our project using the default calling convention. The `noalias` attribute is now added to all the flattened arguments, including those that are not pointers. LLVM complaints about it:


You should not be using `llvm.noalias` with the default calling convention. It has not worked before, since you cannot attach `noalias` to structures either. So I will be surprised if you actually had a test exercising the default calling convention with `llvm.noalias` that used to pass and that was broken by this change.

> This is extending the test case to cover this new scenario:
> 
>   diff --git a/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir b/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir
>   index 105a80dec7a..da3ae1341f0 100644
>   --- a/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir
>   +++ b/mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir
>   @@ -2,6 +2,9 @@
>    // RUN: mlir-opt -convert-std-to-llvm='use-alloca=1' %s | FileCheck %s --check-prefix=ALLOCA
>    // RUN: mlir-opt -convert-std-to-llvm='use-bare-ptr-memref-call-conv=1' -split-input-file %s | FileCheck %s --check-prefix=BAREPTR
>   
>   +// CHECK-LABEL: func @check_noalias
>   +// CHECK-SAME: %{{.*}}: !llvm<"float*"> {llvm.noalias = true}, %{{.*}}: !llvm<"float*"> {llvm.noalias = true}, %{{.*}}: !llvm.i64, %{{.*}}: !llvm.i64, %{{.*}}: !llvm.i64,
>   +// CHECK-SAME: %{{.*}}: !llvm<"float*"> {llvm.noalias = true}, %{{.*}}: !llvm<"float*"> {llvm.noalias = true}, %{{.*}}: !llvm.i64, %{{.*}}: !llvm.i64, %{{.*}}: !llvm.i64)
>    // BAREPTR-LABEL: func @check_noalias
>    // 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}) {

This is wrong. The allocated and aligned pointer _do_ alias, so attaching "noalias" to them is incorrect. There is no aliasing model on memrefs. When we have one, we will see how to lower them to the LLVM dialect.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74211/new/

https://reviews.llvm.org/D74211