This separation improves the layering and paves the way for more interfaces coming up in the future.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) | Shouldn't this file be under include/mlir/Dialect/Linalg/... ? |
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) | Hmm I went through some effort moving it under mlir/Interfaces because virtually all interfaces live there (or in IR but not applicable here). Is there a strong opinion that this should not live here? |
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) | +1 to what Medhi says. Dialect specific interfaces should live near those dialects, this directory is intended for more general things. Affine already sets a precedent here (and a good one): https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Affine/IR/AffineMemoryOpInterfaces.td |
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) | Something that isn't clear to me conceptually is why does it make sense to have an interface that is specific to a Dialect? In general we introduce interfaces to decouple between what the client (a pass/analysis) wants to know and the "object" it operates on. The interface makes sense so that multiple dialects or operation can answer the query. Is it intended that this interface would be implemented by other dialects? If so it can makes sense to have it here then. |
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) |
I don't see how this is different for a single dialect? If my dialect has a bunch of operations that have similar characteristics, why wouldn't I use an interface to make my transformations simpler? Saying that interfaces only make sense inter-dialect brings us back to hardcoding operation names in transformations, which may make sense for some things but it doesn't really scale (whether that be with one dialect or multiple). |
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) |
I wrote multiple dialects OR operation, what I had in mind was: -> Operation Interface: when you have multiple operations (possibly in the same dialect) |
mlir/include/mlir/Interfaces/LinalgInterfaces.h | ||
---|---|---|
44 ↗ | (On Diff #321083) | But I think we're on the same line that if an Operation Interface is intended for the operations in a specific dialect, then it can live within this dialect and not here :) |
clang-tidy: warning: header guard does not follow preferred style [llvm-header-guard]
not useful