[all-commits] [llvm/llvm-project] 499703: Enable ReassociatingReshapeOpConversion with "non-...

[bjacob via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 499703e9c08a055a9007278e5222b0550aba89bf
      https://github.com/llvm/llvm-project/commit/499703e9c08a055a9007278e5222b0550aba89bf
  Author: Benoit Jacob <benoitjacob at google.com>
  Date:   2022-01-13 (Thu, 13 Jan 2022)

  Changed paths:
    M mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
    M mlir/test/Conversion/MemRefToLLVM/memref-to-llvm.mlir

  Log Message:
  -----------
  Enable ReassociatingReshapeOpConversion with "non-identity" layouts.

Enable ReassociatingReshapeOpConversion with "non-identity" layouts.

This removes an early-return in this function, which seems unnecessary and is
preventing some memref.collapse_shape from converting to LLVM (see included lit test).

It seems unnecessary because the return message says "only empty layout map is supported"
but there actually is code in this function to deal with non-empty layout maps. Maybe
it refers to an earlier state of implementation and is just out of date?

Though, there is another concern about this early return: the condition that it actually
checks, `{src,dst}MemrefType.getLayout().isIdentity()`, is not quite the same as what the
return message says, "only empty layout map is supported". Stepping through this
`getLayout().isIdentity()` code in GDB, I found that it evaluates to `.getAffineMap().isIdentity()`
which does (AffineMap.cpp:271):

```
  if (getNumDims() != getNumResults())
    return false;
```

This seems that it would always return false for memrefs of rank greater than 1 ?

Reviewed By: nicolasvasilache

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