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 | ||
---|---|---|
295–299 | style | |
373 | 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) | |
422 | Excellent! I've been wanting to factor this function out of fromCOO for a while, just hadn't gotten around to it yet :) | |
436–437 | 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. | |
463 | dimension. |
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp | ||
---|---|---|
373 | Agreed, more readable in the long run and it will trigger the singleton work changes needed. | |
422 | We probably can refactor more shared code, but this was rather tricky to get right, so let's tread carefully. ;-) | |
436–437 | I don't disagree, but ending at zero with an unsigned made the iteration a bit awkward.... |
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp | ||
---|---|---|
436–437 | 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 | ||
---|---|---|
436–437 | Added the assert. |
style