Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.cpp | ||
---|---|---|
61 | This function looks more like fromSliceCoord to me, where "v" is the coordinate in the slice view. Am I right? | |
66 | iv = slice_iv * stride + offset? I am not sure whether we should also add a period in this case? | |
72 | Related to the above, this looks like toSliceCoord to me. | |
78 | slice_iv = (iv - offset)/stride? | |
359 | confirm? | |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp | ||
1013 | Converter? | |
1168–1176 | I don't see FileCheck test for this pattern, can we add such test? | |
mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_foreach_slices.mlir | ||
6 | Do we really need this lib? | |
50–53 | We should avoid having the second part repeat the first part, maybe even try to avoid having the same NZ values in two different positions. |
address comments.
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.cpp | ||
---|---|---|
61 | Hmmm, yeah, updated. | |
359 | No, I confirmed that it is folded. But it does not really matter, I manually skip the check anyway in the following revision. | |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp | ||
1168–1176 | This is a nop right now (the check test is added in the dynamic slice revision, which actually implements it). |
mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp | ||
---|---|---|
540 | so what happens if you do "the right thing" here already? | |
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.cpp | ||
61 | Typo: Converts | |
62 | tensor (singular)? | |
74 | Typo: Converts | |
349 | Using numbers and code is hard to read. First, coord >= ... | |
381 | insertion point | |
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.h | ||
251 | needs its own comment, or be moved some place else? | |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp | ||
1025 | TODO, not "probably" ;-) |
mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp | ||
---|---|---|
540 | I remember that it crashed, as there is no fields for dimSliceAttr in specifier yet. |
so what happens if you do "the right thing" here already?