[PATCH] D74389: [MLIR][GPU] Disallow llvm tanh intrinsics when lowering to NVVM/ROCm.

[Stephan Herhut via Phabricator via llvm-commits]

herhut marked an inline comment as done.
herhut added a comment.

In D74389#1868977 <https://reviews.llvm.org/D74389#1868977>, @ftynse wrote:

> This would work for me. At scale, we may need to analyze effects of such approach on compile time. The converter essentially twice the number of patterns it has to search through, and there are potential rollbacks of rewrites that generated illegal operations.
>
> In general, I think we want to expose individual LLVM patterns anyway because several people hit the selection problem repeatedly. I haven't had time to do that yet.


The granularity of exposing them is not clear to me. I also thought about an intrinsics/non-intrinsics split. But even the definition of what intrinsic based lowering means is vague. LLVM::ExpOp is an operation but ultimately lowered to some intrinsic. One way to make this cleaner would be to expose all intrinsics as LLVM::Operation in MLIR and only lower them when going to llvm proper. Then at least we can filter on operations and not intrinsic names.

> For intrinsics specifically, I have a vague idea of some fallback mechanism, as in: the default Std->LLVM-intrinsic patterns are enabled unless there is a different pattern.

It would be great if one could define priorities of patterns so that certain pattern shadow others. This is different from benefit in that it would have stronger semantics (disallowing patterns rather than preferring them). One idea would be to group pattern into formal groups and then allowing to specify a partial order on these groups, which is then used to specify application order of patterns. That would solve out case where NVVM pattern are meant to shadow LLVM pattern.

For now, should we land this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74389/new/

https://reviews.llvm.org/D74389