linalg.generic ops have canonicalizers that either remove arguments
not used in the payload, or redundant arguments. Combine these and
enhance the canonicalization to also remove results that have no use.
This is effectively dead code elimination for Linalg ops.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp | ||
---|---|---|
16 | Do you need this? | |
21 | Same question as above. | |
705 | There is a helper function called hasUniqueDim on line 114 that is not needed with this logic, so you can delete that. | |
710 | is is -> is | |
mlir/test/Dialect/Linalg/canonicalize-duplicate-inputs.mlir | ||
94 | I believe as part of adding the tests here, you can delete two tests in canonicalize.mlir |
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp | ||
---|---|---|
700 | I think this can be put in the function. | |
705 | inversePermutation() asserts on that the indexing map does not have a symbolic expression. We might need an explicit check for that. | |
mlir/test/Dialect/Linalg/canonicalize-duplicate-inputs.mlir | ||
108 | Q: do we expect a race condition as a desirable outcome? |
Address comments and disallow dropping of dead outputs when it is needed for loop dim computation.
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp | ||
---|---|---|
16 | Ah thanks. Dont need it anymore. I thought I did. | |
21 | This I do need. | |
700 | Wont it result in a allocation on every invocation? | |
705 | LinalgOp verify checks for no symbols. So it can be an assumed property. In general this computation is how loop bounds are determined based on indexing maps. | |
mlir/test/Dialect/Linalg/canonicalize-duplicate-inputs.mlir | ||
94 | Should be fine. More tests are not a problem :) | |
108 | Not sure I follow the question. How is there a race condition here? |
mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td | ||
---|---|---|
722 ↗ | (On Diff #422391) | All the uses I found are actually calling getValue immediately to get the AffineMap. I would thus suggest to use the existing getTiedIndexingMap function to avoid growing the interface? If an attribute is needed then AffineMapAttr::get() is pretty compact to get it? |
mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td | ||
168 | nit: resultTensorTypses -> resultTensorTypes | |
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp | ||
691 | nit: in most places we prefix the operands of the newly created op with new instead of modified. Feel free if you think modified is a better fit here! | |
721 | I think it would be nice if we could avoid the assumption and rely on the structured op interface only? We could for example use a slightly different algorithm that for every loop dimension counts the number of indexing map result appearances. E.g. if an op has the indexing maps (d0, d1) -> (d0, d1) and (d0, d1) -> (d0) we could get the vector dimCounts = [2, 1] saying d0 appears twice and d1 once. We can then iterate opOperands and and try to subract the indexingMap tied to the current opOperand. If the dimCounts vector contains a zero after the subtraction we cannot delete it. In the example, only the operand tied to (d0, d1) -> (d0) can be removed. I think if we have two lambdas/functions to add and subtract the result dim counts for a specific indexing map the change may even simplify the pattern a little bit. Admittedly, it also implements some logic that is usually hidden in getShapesToLoopsMap (the real one in the structured op interface). An alternative could be to have a method on the structured op interface that takes a list of dropped operands and returns if the remaining ones suffice to define the iteration domain E.g.: genericOp.isIterationDomainDefinedBy(/*droppedOperands*/=SmallVector<OpOperand*>{opOperandToDelete1, opOperandToDelete2}); which could then be used in the verification as well. That could fit well with the current algorithm. | |
843 | I think it should be possible to use rewriter.replaceOp(genericOp, newResults) by setting the dropped values to the old result or nullptr? Or is there some check that prevents this? |
mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td | ||
---|---|---|
722 ↗ | (On Diff #422391) | You are right. I dont need this anymore. |
mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td | ||
168 | Seems like Gollum came and wrote that signature :) | |
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp | ||
691 | Good idea. Smaller names. | |
721 | The first approach will require walking the indexing maps, and maintaining involved data structures to get it current. Seems more invasive and error prone. The second approach looks feasible. Let me try that. |
Use interface method to check if operands can be dropped while still being able to compute loop bounds.
nit: resultTensorTypses -> resultTensorTypes