[Mlir-commits] [mlir] [mlir][TilingInterface] Add scf::tileUsingSCFForallOp method to tile using the interface to generate `scf::forall`. (PR #67083)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > @nicolasvasilache I adapted the implementation here from `linalg::tileToForAll`.
> 
> Great to see this! I am hopeful this will also let us clean up a bunch of old stuff that I couldn't do earlier, in particular to avoid downstream breakages (notice in the first commit one of the TODOs I had mentioned re. tile reduction to parallel and the general need to rewrite that part).
> 
> See a rough sketch here of stuff I'd like to drop https://github.com/nicolasvasilache/llvm-project/tree/tiling-cleanups, do you think this is doable or is there something load-bearing here ? If something load-bearing remains, could we take needed subsets downstream ? Also @stellaraccident for visibility.

Thanks! yeah those should be retire-able in due course. Ill keep these in mind.

> 
> > 2. Is there a difference for the `tile_size` mode and the `num_threads` mode here?
> 
> They have different behaviors in cases that don't divide: num_threads guarantees a static number of threads and makes the tile sizes dynamic. If we only specified tile sizes and things don't divide we could end up with dynamic number of threads which has quite some implications later. This was introduced here https://reviews.llvm.org/D130139 (see e.g. the tests in that PR) and the logic around `tileToForallOpUsingTileSizes` and `tileToForallOp`.

I am trying to basically see if I can standardize on the tile_size variant.. That is consistent with the tiling using scf.for... Maybe some more context would help. For example, `num_threads` could still be dynamic number of threads where the `num_threads` is dynamic... Maybe worth catching up more on GVC here.

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