Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.h | ||
---|---|---|
202–203 | just an idea for later, but do we want to increase # tensors by one also? | |
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp | ||
1496 | typo: synthetic | |
1593 | yeah, this is good logic | |
mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_reductions_prod.mlir | ||
144 | repeating Wren's feedback on the typo originating from the other revision: "particular" |
address comments.
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.h | ||
---|---|---|
202–203 | Yeah! We should! Also it now matches the number of tensor in Merger. But I will do it in a separate patch... |
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.h | ||
---|---|---|
202–203 | I think it'd be good to have the getNumTensors method give the +1 result; so that it stays consistent with the Merger method of the same name, as well as avoiding needing to spell out all these off-by-one cases | |
420 | Moving this assertion defeats the purpose. We need to check the validity of tid before evaluating collapseReassoc[tid] |
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.cpp | ||
---|---|---|
235–238 | Nit: I think it'd be better to avoid saying "real" here, since that evokes the mathematical meaning of "real" meaning that the elements are real-numbers. Often the antonym of "synthetic" is "analytic", though I don't think that's appropriate either (since it evokes other logical/mathematical things). Hmm... I would suggest "numOperands", but this includes the output tensor too... Perhaps, "numManifestTensors"? |
just an idea for later, but do we want to increase # tensors by one also?