Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
389 | Just think out loud, I somehow feel that you can reuse (or extend current collapse/expand_shape) here. For example, extend reshapeCvs to take two reassociations, one for collapse and one for expand. collapse_shape/expand_shape only have one reassociation set, while reshape has both set. So that you can merge the three rewriters into similar implementation. |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
349 | super nit, all rules here have a comment of the form /// Sparse rewriting rule for the xxx operator. that just describes the operator, but does not repeat the exact type found in the template following anyway | |
367 | can't we combine this with L361, before we collect the sizes? | |
375 | an unordered | |
387 | This block of code needs a leading comment that shows some pseudo-code on what construct is being generate | |
402 | These comments really do not help much without a leading comment that describes what is happening |
This looks good with the modifications. One last request, can you also add a CHECK test on the generated pattern. It does not have to test all the details, but at least the resulting loop structure to some extend?
mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_reshape.mlir | ||
---|---|---|
19 | do we really need to test (VLA) vectorization here too? |
mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_reshape.mlir | ||
---|---|---|
19 | Removed vectorization test |
super nit, all rules here have a comment of the form
/// Sparse rewriting rule for the xxx operator.
that just describes the operator, but does not repeat the exact type found in the template following anyway