The start value will be used during reductions instead of assuming that
the initial value is zero, allowing e.g. multiplication during
a reduction which needs 1.0 as the starting value rather than 0.0.
Details
- Reviewers
aartbik
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I plan to add custom reduction using sparse_tensor.binary later. This PR will focus solely on explicit starting values for non-custom reductions.
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td | ||
---|---|---|
77 | I am unhappy with the need to specify the type of startValue. It should always be the same as the result's element type. However, I tried several iterations of TypesMatchWith trait and custom builders and couldn't get it to work. I think it is because startValue is optional, so the inferring of type is tricky. Requiring the type to be specified works, but if someone knows how to eliminate this, I think it would look better. |
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td | ||
---|---|---|
64 | replace "the" before unitialized with "an" | |
66 | Perhaps give a small motivation for this | |
77 | I can't think of an easy way around either (others?). But I think it is okay, MLIR is not really a programming language in the sense that our syntax needs to be as concise as possible. Sometimes we over state the types. | |
mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp | ||
236 | this will need an invalid.mlir case also, the new syntax needs an roundtrip.mlir |
Note that there is this parallel effort to unify our tensor initialization ops (see https://reviews.llvm.org/D126180).
So let's brainstorm offline to come up with another solution.
With the removal of sparse_tensor.init operation, this change no longer makes sense.
@aartbik and I discussed an alternative to create a new sparse_tensor.reduce operation which will accept a start value.
replace "the" before unitialized with "an"