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 | ||
|---|---|---|
| 36 | 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 | ||
| 26 | Super-nit: start error messages with a non-capital letter, these are usually appended to something.  | |
| 42 | 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 | ||
|---|---|---|
| 42 | Yeah, that is what we expect, I will keep it.  | |
| mlir/test/Integration/Dialect/SparseTensor/TransformOps/match_sparse_ops.mlir | ||
|---|---|---|
| 1 ↗ | (On Diff #547257) | 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 | ||
|---|---|---|
| 1 ↗ | (On Diff #547257) | I think it make sense to move it out integration test.  | 
| mlir/test/Integration/Dialect/SparseTensor/TransformOps/match_sparse_ops.mlir | ||
|---|---|---|
| 1 ↗ | (On Diff #547257) | Thanks, feels better to me too.  | 
looks like this is a copy and paste error from the original file that inspired you ;-)