First version was vectors only. With some clever "path" insertion,
we now support any d-dimensional tensor. Up next: reductions too
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp | ||
---|---|---|
294–298 | style | |
372 | I like removing the need for the sparsity parameter. But for legibility/clarity, add a method bool isCompressedDim(uint64_t) to encapsulate this idiom everywhere it's used. (And/or bool isDenseDim(uint64_t) depending on how you expect Singleton and other DimLevelTypes to go) | |
421 | Excellent! I've been wanting to factor this function out of fromCOO for a while, just hadn't gotten around to it yet :) | |
435–436 | Since the goal of that definition for d is to reverse the iteration order and i is used nowhere else, it would be cleaner to just iterate d directly. | |
449 | dimension. |
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp | ||
---|---|---|
372 | Agreed, more readable in the long run and it will trigger the singleton work changes needed. | |
421 | We probably can refactor more shared code, but this was rather tricky to get right, so let's tread carefully. ;-) | |
435–436 | I don't disagree, but ending at zero with an unsigned made the iteration a bit awkward.... |
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp | ||
---|---|---|
435–436 | Doh, I missed that. I still think phrasing things as counting down makes more sense though. Could always rephrase things as a while-loop... uint64_t rank = getRank(); assert diff < rank; uint64_t d = rank - 1; while (true) { ...stuff... if (d == diff) break; --d; } Though that does start looking pretty gross too. Actually, while writing that up I just noticed you'll want to have that assertion regardless of whether rephrasing the loop, since otherwise rank - diff will do bad things. |
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp | ||
---|---|---|
435–436 | Added the assert. |
style