Perhaps we can retire linalg_ext.reverse op? #16060
Comments
hanhanW
added
good first issue 🌱
@TSWorld1314 sorry for the confusion. @hanhanW what is the issue with using reverse? Is it that it doesnt get fused with things? Thats valid. Can then verify that using |
|
I mainly want to start a discussion about whether we want to use linalg_ext.reverse or not. I observed that the torch frontend and stablehlo frontend have different lowering behavior; I think we can revisit the needs of linalg_ext.reverse. I can see that the op stops fusion and vectorization, which might can be improved. I think we can vectorize tensor.extract and use it in IREE, but we will need someone to verify. (I can do it, but I don't find cycles. Because this is a fairly low priority at this moment) If it works better, we can consider to drop the linalg_ext.reverse op. It will allow us to maintain less LinalgExt ops. |
@TSWorld1314 maybe you can help look into it |
@MaheshRavishankar @hanhanW ，hello，I'm truly excited about tackling this verification task, but I have a few questions. Could you provide me with an example of a stablehlo frontend? Writing stablehlo isn't my strong suit yet, but I'm eager to learn. Also, I'm unsure if my code is correct; it might be necessary to compare the generated IR for verification. How can I use iree tools to generate stablehlo IR? I'm thrilled to have this opportunity to work on this task and help resolve this issue. Any advice on learning IREE would be greatly appreciated! |
|
@TSWorld1314 there are good introductions about how to contribute to IREE and testing. If you want to learn more about IREE before jumping into the task, please take a look at the https://iree.dev/developers/ Thanks |
|
@hanhanW I am really thankful for your reply. I will learn it quickly and then try to write an example for verification. I am really grateful to you! |
|
If you are looking for an example input IR, here is one: https://github.com/openxla/iree/blob/e2e126ce061454ad71bc2f5c1c08b1efc982f4cd/compiler/plugins/input/StableHLO/stablehlo-iree/Conversion/test/stablehlo_to_linalg_ext.mlir#L489-L502 But it looks like you are not familiar with IREE and testing, so I suggest to study https://iree.dev first. |
I'm truly grateful for your help. I'll start learning it as soon as possible! Thanks a lot, hanhanW |
|
If we can confirm that this operation is vectorized on the CPU backend then we can drop this operation. One way to confirm is to compile the above example with The Actually as I write this |
|
hello, @hanhanW @MaheshRavishankar it results in an error log: My input mlir is: And I hope replace IREE::LinalgExt::ReverseOp. |
|
I think you can compare your implementation with https://github.com/llvm/llvm-project/blob/a9c5bddc8f18926bac6dc224144a32512207bd38/mlir/lib/Conversion/TosaToLinalg/TosaToLinalg.cpp#L1736-L1792 The above log indicates that it crashes in op builder. So here is a bug in line 475: Value extractedElement = nestedBuilder.create<tensor::ExtractOp>(nestedLoc, args[0], indices);
|
|
hello @hanhanW ， And input mlir is： this is output: Additionally, how can I demonstrate improvements over the previous version with some examples? I'm truly grateful for your assistance! |
|
@TSWorld1314 there are no progress for months, so I assume that you don't work on it anymore. |
|
@lialan I think this is a good starter task. One of expectation is to make sure that the op is vectorized. You can verify it with You can use https://github.com/iree-org/iree/blob/main/tests/e2e/stablehlo_ops/reverse.mlir as the input for iree-compile. The command would be something like |
Okey, thanks hanhanW. |
`LinalgExt::ReverseOp` is only lowered from `stablehlo::ReverseOp`. We can expand `stablehlo::ReverseOp` to a different pattern and retire `LinalgExt::ReverseOp`. Fixes iree-org#16060 --------- Signed-off-by: Alan Li <[email protected]> Signed-off-by: Lubo Litchev <[email protected]>
I found that in tosa.reverse lowering, we are not using linalg_ext ops. It is using
linalg.indexandtensor.extractops to gather the element. We can use the same pattern in stablehlo -> iree input conversion. If we do so, linalg_ext.reverse op won't have any users; we can retire the op.The text was updated successfully, but these errors were encountered: