[Mlir-commits] [mlir] [mlir][transform] Fix new interpreter and library preloading passes. (PR #69190)

[Oleksandr Alex Zinenko]

Ingo =?utf-8?q?Müller?= <ingomueller at google.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/69190/mlir at github.com>


ftynse wrote:

> Thanks for the review. Nits addressed in latest commit.
> 
> > Could you elaborate why the dialect dependency is necessary for these passes? It is usually required when the pass creates entities from the dialect, are these passes creating something?
> 
> Sure:
> 
> * The preload pass calls [`MLIRContext::getOrLoadDialect<transform::TransformDialect>`](https://github.com/llvm/llvm-project/blob/1bf08709/mlir/lib/Dialect/Transform/Transforms/PreloadLibraryPass.cpp#L35C22-L35C69).
> * The interpreter pass [calls `transform::detail::getPreloadedTransformModule`](https://github.com/llvm/llvm-project/blob/4606712ef5b422edbe3799b665dcad7dcf348b90/mlir/lib/Dialect/Transform/Transforms/InterpreterPass.cpp#L30), which [does the same](https://github.com/llvm/llvm-project/blob/1bf08709/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp#L115C16-L115C63).

I see, thanks. What really escaped my attention here is that a pass cannot load a dialect because it run in multithreaded mode, so it has to declare as dependent any dialect it needed to load and have it loaded by the pass manager. This is different from creating entities from the dialect, which is impossible without the dialect being loaded in the first place. Not sure if this is documented sufficiently, somehow it wasn't obvious to me.

> I don't think it was approved though, was it?

Indeed, it wasn't. I haven't requested changes as I was rather asking for clarification. Approved after-the-fact, but generally better to wait for approval in the future.

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