Add the makeComposedExtractSliceOp method that creates an ExtractSliceOp and folds chains of ExtractSliceOps by computing the sum of their offsets and by multiplying their strides.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Just a few comments.
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
207 | I have written this function multiple times. Need a common place for it. I think its complicated since it requires standard dialect so any place this is put needs to also add dependency on standard dialect. | |
220 | Not sure doing this recursively in a loop is a good idea. Maybe just do one step and allow the pattern rewriter to apply it multiple times? | |
222 | Why? This is just folding the offsets and the strides. The type should still remain the same right. So you should be able to handle this? | |
229 | I am not convinced that you can just add the offsets. I think you can do that only if the strides are 1 (though I havent given this deep thought, just a first impression). I would just simplify all of this and check for all strides in the producer and consumer to be unit strides and bail if they arent. I dont think we have a case with strides not being 1 anyway. Meta point: We should just drop strides from subviews. Do we ever use non-unit strides? |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
207 | I tried to reuse an existing implementation but all of them introduce strange dependencies. Also a standard dialect implementation probably requires more functionality in the sense that we would have to support non integer types etc. | |
222 | The idea is to make sure the producer is not rank reducing. It it is there would be a type mismatch. | |
229 | Good point! I think you are right regarding the strides and adding offsets. The two offsets can potentially have different strides which cannot be handled properly in one extract slice. I will check for stride one then. |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
207 | Just fyi. There is getAsOpFoldResult etc. in StaticValueUtils, which are related to this. I ran into the same issue with "dependency on std" when I added these, which is why I couldn't add this function. Anyway... I don't know how to solve this in a nice way either. |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
207 | StandardOps/Utils/Utils.h may be an option? But then the idea seems to be to remove std... |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
204 | can you early exit and avoid increasing nesting logic? | |
210 | Just checking equality of the strides of the 2 ops would be enough here. Re. "do we need something else than stride 1": in practice today no but things like dilation in convolutions are one of the envisioned use cases. | |
224 | Could you provide a version of makeComposedAffineApply that works with OpFoldResult and simplify the caller? |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
210 | After some more thought I think the relevant information are the producer strides? If they are one then both offsets are expressed using the same strides. | |
224 | After some more though I think my last version was wrong as well. If I am not mistaken the best solution for now is to check the producer strides are one. I think this is a necessary condition to guarantee the producer and consumer offsets are expressed in the same stride? In theory we could also handle other producer strides by multiplying the consumer offset by the producer stride before adding the offsets for the folding. Or am I wrong again here? |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
210 | Ah yes my apologies .. strides in subX operations compose (i.e. the op can strictly subsample its input). Resulting strides should be the product of strides. |
mlir/lib/Dialect/Linalg/Utils/Utils.cpp | ||
---|---|---|
210 | Sigh .. I meant: Result offset should be producer_offset + consumer_offset * producer_strides. Here is the code that implements the lowering to LLVM (replace producer by "sourceMemRef"): Now as Mahesh pointed out, this is not well tested on actual use cases so maybe failing if strides are != 1 is the most pragmatic thing to do right now .. |
can you early exit and avoid increasing nesting logic?