-
Notifications
You must be signed in to change notification settings - Fork 359
Use output shape when lowering Unique to krnl #2820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Tong Chen <[email protected]>
Signed-off-by: Tong Chen <[email protected]>
Signed-off-by: chentong319 <[email protected]>
This reverts commit ebd27e2.
This reverts commit 432e4be.
Signed-off-by: chentong319 <[email protected]>
Signed-off-by: chentong319 <[email protected]>
@chentong319 Your fixes look good to me! Thank you so much for fixing lowering Unique to krnl. |
Could you approve the PR? |
@chentong319 Since @negiyas said this looks good, I will make sure to merge this in for you :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Jenkins Linux s390x Build #14814 [push] Use output shape when lo... started at 14:48 |
Jenkins Linux ppc64le Build #13839 [push] Use output shape when lo... started at 15:01 |
Jenkins Linux amd64 Build #14809 [push] Use output shape when lo... started at 13:48 |
Jenkins Linux amd64 Build #14809 [push] Use output shape when lo... passed after 1 hr 14 min |
Jenkins Linux s390x Build #14814 [push] Use output shape when lo... passed after 1 hr 46 min |
Jenkins Linux ppc64le Build #13839 [push] Use output shape when lo... passed after 2 hr 0 min |
This issue is discovered by #2799 . It may not be considered a bug in lowering. The issue is triggered by two imperfect components: onnx backend test and buffer deallocation.
onnx backend test
The output shape of Unique OP depends on the content of the input tensor. But the onnx backend test creates the onnx model based on the chosen input. Therefore, the output shape is static. I opened an issue in onnx.
Buffer deallocation
Due to the mismatch of the generated and specified type of the outputs UniqueOp, the existing lowering of Unique created some unrealized_conversion_cast ops. The deallocation pass in the new llvm can not handle the conversion for returned tensors correctly.
Anyway, I try to fix the problem from onnx-mlir with this PR. I use the type of the outputs for allocation if the type is static.