Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
---|---|---|
246–254 | It is actually prepared for filter loop idx (which is not yet introduced in this patch). But I keep it here to avoid merge conflict. |
mlir/include/mlir/Dialect/SparseTensor/Utils/Merger.h | ||
---|---|---|
155 | add comment on filter loops | |
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
99 | move data to end (just the way google3 style works make it private | |
1215 | here and below, this is very subjective but I like // Handle xxx return .... or if (...) return ... // handle x a bit better than if (...) // Handle x. return if there are no braces. But since there is no style for this, your choice in the end |
mlir/include/mlir/Dialect/SparseTensor/Utils/Merger.h | ||
---|---|---|
155 | Filter loops are not yet introduced. Will add the comment in that patch to be more consistent. |
mlir/include/mlir/Dialect/SparseTensor/Utils/Merger.h | ||
---|---|---|
155 | Yeah, not sure why i said that ;-) | |
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
98 | please add a bit of doc to the class, especially since the method names are a bit obscure, e.g. setPickedIterType does not have direct meaning on who picks what ;-) | |
258 | please doc what expansion means here | |
324 | this is a huge block of code inside the iteration graph computation, can we move this into own function with intuitive name so that the original idea of AffineExpr f = map.getResult(toOrigDim(enc, d - 1)); AffineExpr t = map.getResult(toOrigDim(enc, d)); addAffineOrderings(adjM, inDegree, f, t, 0); is preserved? Otherwise it is very hard to separate intended logic from heuristic addition | |
587–590 | this Note that comment is obsolete too now, right, or at least need some adjustmnet |
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
---|---|---|
324 | The reason why I started from 0 is actually due to filter loops... because the filter loop constraints are not imposed between (d - 1, d) but on (d) alone. Maybe I should do the refactoring on the patch where filter loop is introduced? |
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
---|---|---|
324 | Ok, put a TODO here and then I accept the future promise ;-_ |
Messages crossed, but except for the typo, LGTM stands
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
---|---|---|
289 | typo Constraints |
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
---|---|---|
587–590 | nice catch! |
add comment on filter loops