[Mlir-commits] [mlir] [mlir][TilingInterface] Use `LoopLikeOpInterface` in tiling using SCF to unify tiling with `scf.for` and `scf.forall`. (PR #77874)

Nicolas Vasilache llvmlistbot at llvm.org
Fri Jan 12 08:47:27 PST 2024


https://github.com/nicolasvasilache requested changes to this pull request.

This is the followup from #72178, right?
Let's mention it in the commit message so we can refer back to it later, had to do a bit of spelunking to find it.

Overall comment:

So IIRC, #72178 adopted the logic of tiling `scf.forall` from `LinalgTransformOps.cpp` and/or `Linalg/Transforms/Tiling.cpp` to redo `scf.for`.

This was a welcome refactoring and this PR is a nice continuation, nice!

To proceed confidently, I would now like to see red diffs in `LinalgTransformOps.cpp` and `Linalg/Transforms/Tiling.cpp` to see older implementations be actually replaced: atm this seems it is still adding more code in parallel to the old implementation.

For instance, I still see LinalgTransformOps.cpp calling code from 
```
  FailureOr<linalg::ForallTilingResult> maybeTilingResult = failure();
  if (!mixedNumThreads.empty()) {
    maybeTilingResult =
        linalg::tileToForallOp(rewriter, tileableOp, mixedNumThreads, mapping);
  } else {
    maybeTilingResult = linalg::tileToForallOpUsingTileSizes(
        rewriter, tileableOp, mixedTileSizes, mapping);
  }
```

Do you have a followup PR which cleans up the old?

Note, as part of this, I would also like to see something concrete on the `num_threads` discussion.

Besides the meta-comment that we now need to resolve, I'll be able to review this early next week.

Thanks!

https://github.com/llvm/llvm-project/pull/77874


More information about the Mlir-commits mailing list