[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