Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| mlir/include/mlir/Dialect/SparseTensor/TransformOps/SparseTensorTransformOps.td | ||
|---|---|---|
| 28 | I was in the process of making some ill-advised suggestions on how the transform should be more composable but then I realized that this is a actual match op and not a transform. So I will defer to @ftynse as I have not yet had time to dig into that part of the system. | |
typo in title: instroduce
| mlir/include/mlir/Dialect/SparseTensor/TransformOps/SparseTensorTransformOps.h | ||
|---|---|---|
| 35 | looks like this is a copy and paste error from the original file that inspired you ;-) | |
| mlir/include/mlir/Dialect/SparseTensor/TransformOps/SparseTensorTransformOps.td | ||
|---|---|---|
| 2 | should adjust this line to fit the 80-char limit | |
| mlir/include/mlir/Dialect/SparseTensor/TransformOps/SparseTensorTransformOps.td | ||
|---|---|---|
| 35 | Nit: this can use custom<SemiFunctionType>(type($target), type($result)) if desired so it is future-proof with potential syntax changes in other match ops. | |
| mlir/lib/Dialect/SparseTensor/TransformOps/SparseTensorTransformOps.cpp | ||
| 25 | Super-nit: start error messages with a non-capital letter, these are usually appended to something. | |
| 41 | It doesn't (yet?) generate anything, so there's no need to declare this. Okay to keep if there is an intention to add more ops quickly. | |
| mlir/lib/Dialect/SparseTensor/TransformOps/SparseTensorTransformOps.cpp | ||
|---|---|---|
| 41 | Yeah, that is what we expect, I will keep it. | |
| mlir/test/Integration/Dialect/SparseTensor/TransformOps/match_sparse_ops.mlir | ||
|---|---|---|
| 2 | Perhaps others feel strongly about this, but can we simply put this in test/Dialect/SparseTensor/TransformsOps (i.e. not as an integration test). It feels different from the end-to-end run that we define there | |
| mlir/test/Integration/Dialect/SparseTensor/TransformOps/match_sparse_ops.mlir | ||
|---|---|---|
| 2 | I think it make sense to move it out integration test. | |
| mlir/test/Integration/Dialect/SparseTensor/TransformOps/match_sparse_ops.mlir | ||
|---|---|---|
| 2 | Thanks, feels better to me too. | |
looks like this is a copy and paste error from the original file that inspired you ;-)