[mlir] Bug in MemrefToLLVM lowering pattern for memref.transpose

[ubfx]

Permutation maps can sometimes be confusing because it's easy to mix up the source and the target dimensions. I think this is what happened with the lowering Pattern for memref.transpose:
https://github.com/llvm/llvm-project/blob/main/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp#L1562

For the testcase to demonstrate the problem I will use the textual representation of verified Ops where it is clear what the shape and strides should be as the reference:

test.mlir:

func.func @transpose(%arg0: memref<1x2x3xf32, strided<[6, 3, 1], offset: ?>>) -> memref<3x1x2xf32, strided<[1, 6, 3], offset: ?>> {
  %0 = memref.transpose %arg0 (i, j, k) -> (k, i, j) : memref<1x2x3xf32, strided<[6, 3, 1], offset: ?>> to memref<3x1x2xf32, strided<[1, 6, 3], offset: ?>>
  return %0 : memref<3x1x2xf32, strided<[1, 6, 3], offset: ?>>
}

test.cpp:

#include <mlir/ExecutionEngine/CRunnerUtils.h>
#include <iostream>

extern "C" void _mlir_ciface_transpose(StridedMemRefType<float, 3> *, StridedMemRefType<float, 3> *);

int main() {
    StridedMemRefType<float, 3> in, out;

    in.sizes[0] = 1;
    in.sizes[1] = 2;
    in.sizes[2] = 3;
    in.strides[0] = 6;
    in.strides[1] = 3;
    in.strides[2] = 1;
    _mlir_ciface_transpose(&out, &in);

    std::cout << "output has sizes: [" << out.sizes[0] << ", " << 
        out.sizes[1] << ", " <<  out.sizes[2] << "] strides : [" << out.strides[0] << ", " << 
        out.strides[1] << ", " << out.strides[2] << "]\n";
    return 0;
}

$ mlir-opt --normalize-memrefs --memref-expand --finalize-memref-to-llvm --llvm-request-c-wrappers --convert-func-to-llvm --reconcile-unrealized-casts --llvm-legalize-for-export test.mlir -o test.llvm.mlir
$ mlir-translate --mlir-to-llvmir test.llvm.mlir -o test.ll
$ clang++-17 -I/mnt/build_llvm/llvm-project/mlir/include/ test.cpp test.ll
warning: overriding the module target triple with x86_64-pc-linux-gnu [-Woverride-module]
1 warning generated.
$ ./a.out 
output has sizes: [2, 3, 1] strides : [3, 1, 6]

We can see that instead of doing the (i,j,k)->(k,i,j) transposition, the lowering pass actually did it the other way around, i.e. (k,i,j)->(i,j,k).

[ubfx]

Phabricator: https://reviews.llvm.org/D159290

[nicolasvasilache]

It would also make sense to update the affine map representation to a list of integers if you are up for it.
We did a similar change to collapse/expand_shape for the same reason.

Thanks for fixing!

[ubfx]

Good idea! I will have a look and try to apply the integer notation to memref.transpose