[llvm] [mlir] [mlir] Move InlinerInterface from Transforms to Interfaces. (PR #84878)

Christian Sigg via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 14 04:17:55 PDT 2024


chsigg wrote:

> Which exact header and which exact link dependency is a problem?

You are right, the `DialectInlinerInterface` that is used by the dialects is fully implemented in `InliningUtils.h`. So dialects could depend on a header-only target (let's call it `InlinerInterface`). The other declarations in `InliningUtils.h` would only be implemented in the `TransformUtils` target. This has the following problems (purely from a bazel perspective):

- Either `InliningUtils.h` needs to be exported by both `InlinerInterface` and `TransformUtils`. Exporting headers multiple times is bad because it breaks modules and tools don't understand the difference between the two targets.
- Alternatively, export `InliningUtils` only by `InlinerInterface`. This requires all (direct) users of the other declarations depend on both `InlinerInterface` and `TransformUtils`.

I fully understand that we do not want to restructure code to please the bazel build. We don't need to in this particular case (and I haven't looked into similar issues like the `Bytecode` headers yet), so if we agree that the `DialectInlinerInterface` should stay where it is, I can ...

1) Fix this purely in the BUILD files by creating a separate target for `InliningUtils.h/cpp`.

The alternatives would be:

2) This PR in its current form (moving the interfaces into the `Interfaces` directory).
3) Do the same code split as in this PR, but move the files back to the `Transforms` directory.
4) Move only `DialectInlinerInterface` to a separate (self-contained) header file, presumably in the Transforms directory.

If we could agree to one of these (or anything else), that would be great.

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


More information about the llvm-commits mailing list