Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
super neat! no more tuple collision, faster compile-time! nice!
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp | ||
---|---|---|
79–80 | this is no longer Optional, but just LogicalResult, since you have success() at L139 | |
83 | return failure() | |
368 | please keep Location and also the place of this decl (after the comment on // Replace) | |
371–372 | can we give this a better name then cast? I think the original tuple would still make a lot of sense here (as in a,b,c tuple, not tuple(a,b,c)). | |
415 | move below // Construct | |
437 | below // Replace | |
458–459 | tuple? | |
478–479 | tuple? | |
495–497 | tuple? | |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorPasses.cpp | ||
156 | This is again, Everything! | |
186 | commented out? |
Address comments from Aart
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp | ||
---|---|---|
79–80 | This is a callback to the type converter. Return llvm::None tells the converter to try next converting rule. | |
371–372 | I also abstracted the to_value/point/indices to share a SparseGetterOpConverter as the base class to reduce code repetition here. |
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp | ||
---|---|---|
57 | tenors -> tensors | |
79–80 | you are right! | |
502 | Note that in the original rule, I returned failure(), i.e. the rewriting does not occur (which is the idiom used elsehwere too for stuff the sparse compiler cannot handle). We typically generate these ops ourselves, so they are always constant index. But just in case they are not, it seems safer to not touch them. An assert only happens in debug mode, in prod, we will crash on L491 While writing these rules, I was thinking that we should really put in the restriction in the op itself. The variable case *can* be handled, but requires setting up some indexing array to get the field at runtime. Does not seem worth it. | |
519 | Is it possible to let Base::getFieldForOp() return unsigned field. assert(i < fields.size()); return fields[i]; into the base class, and make all derived classes even shorter? |
tenors -> tensors