Previously, we use InsertOp to write the destination tensor. We now directly
store to the destination tensor values buffer. For the case where the source is
a dense tensor and the destination has an identity ordering, we use memref
CopyOp to write the destination.
Details
- Reviewers
aartbik nicolasvasilache Peiming
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
805 | I really feel that we should not allow all dense sparse encoding right now... we should canonicalize it to non-encoded tensor instead. This brings too much unnecessary complexity here. @aartbik WDYT? | |
820 | There if/else here and below are too hard to follow, if we are going to allow all dense encoding, we should still probably split this function up. e.g., genDirectConvert and genIndirectConvert or some other more meaningful methods. |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
805 | We currently distinguish tensors through these methods: |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
820 | Is genDirectConvert for conversion without a tmpCOO and genIndirectConvert for conversion with a tmpCOO? |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
805 | yeah, I understand. But all dense encoded tensor is essentially the same as non-encoded tensor, right? Seems that we are treating allDense as a special case everywhere, which could be avoided. | |
820 | maybe with/without tmpCOO? WDYT? If you want, you can further split it up. |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
805 | The all-dense encoding is really just something that is helpful to debug the sparse codegen. I don't expect clients to ever favor it over truly dense tensors. As such, is the amount of special cases worth it? Can you say a bit more on what it improves? |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp | ||
---|---|---|
805 | For source programs, I think it'd be good to canonicalize all-dense "sparse"-tensors to standard dense-tensors. However, internally, there are a few places where we currently rely on the fact that all-dense encodings are in fact allowed. One example is in the runtime pass's dense2sparse conversionOp (or was it in the sparse2dense?). So if we do want to go the canonicalization route, then it's important to distinguish (1) "all-dense tensors", i.e., tensor values of type RTT with all-dense STEA, (2) the type "RTT with all-dense STEA", (3) the attribute "all-dense STEA". Since just because we canonicalize the first of those three doesn't mean we necessarily canonicalize or invalidate either of the other two. And if we also canonicalize the second one, that still doesn't necessarily mean canonicalizing nor invalidating the third. Also N.B., in D141975 we have code for explicitly going the other direction: allowing us to treat RTT-without-encoding in the same way that we treat RTT-with-STEA. I did that in order to remove a bunch of other special cases in the code. So it's also important to see whether that's sufficient to remove a bunch of the special cases you have in mind, and to ensure that any canonicalization effort doesn't end up introducing special cases somewhere else instead. |
I really feel that we should not allow all dense sparse encoding right now... we should canonicalize it to non-encoded tensor instead. This brings too much unnecessary complexity here. @aartbik WDYT?