This revision retires a good portion of the complexity of the codegen strategy and puts the logic behind pass logic.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
nice simplification!
mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h | ||
---|---|---|
598 | nit: I think it is MatchAnyOpTypeTag but I think I prefer a comment similar to the promotion pattern that just says "entry point to match any LinalgOp ..." | |
817 | nit: I think the second half of the comment got lost. | |
mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp | ||
55 | nit: seems like a corner case but I would probably put braces since the statements are fairly complex. | |
85 | same here | |
115 | same here | |
154 | nit: here I would also put braces on the outer if. |
mlir/include/mlir/Dialect/Linalg/Transforms/CodegenStrategy.h | ||
---|---|---|
87 | nit: LinalgOpType -> LinalgOp since the template parameter is gone. |
mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp | ||
---|---|---|
84 | promotionPattern? |
So much nicer now!
mlir/include/mlir/Dialect/Linalg/Passes.h | ||
---|---|---|
87 | nit: filter instead of filt? opt is used commonly for options, but i am not sure about filt. | |
mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h | ||
597 | is there some special reason not to use OpRewritePattern for LinalgBaseTilingPattern? |
mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h | ||
---|---|---|
597 | Basically history: when this was written, OpRewritePattern did not work with interfaces. Will keep as a followup cleanup or an intro task for someone who has cycles. |
mlir/lib/Dialect/Linalg/Transforms/CodegenStrategy.cpp | ||
---|---|---|
62 | I'm concerned about this: we're not supporting running a pass manager from inside a pass manager: you need to use the dynamic pipeline feature for this: https://mlir.llvm.org/docs/PassManagement/#dynamic-pass-pipelines Can you update this? (we're also increasing the amount of asserts around these situations). |
Actually, I have very strong concern with any Pass hooking to the infrastructure that isn't made "hermetic" : you have passes here that takes arbitrary C++ code here which breaks any pipeline serialization and will generate incorrect crash reproducer.
Didn't we talk about this?
nit: filter instead of filt? opt is used commonly for options, but i am not sure about filt.