Skip to content
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

Fix constant propagation pass issue for onnx.Gather op. #2487

Closed

Conversation

negiyas
Copy link
Collaborator

@negiyas negiyas commented Sep 8, 2023

Fix constant propagation pass issue for onnx.Gather op.
Current constant propagation pass does not work for onnx.Gather with unranked indices (e.g. Tensor).
This PR stops constant propagation with the case.

[X] Prepare LIT test to check errors

onnx-mlir: ./src/Dialect/ONNX/ElementsAttr/DisposableElementsAttr.hpp.inc:75: onnx_mlir::ArrayBuffer<X> mlir::DisposableElementsAttr::getArray() const [with X = long int]: Assertion `onnx_mlir::toBType<X> == getBType() && “X must match element type”‘ failed.

@negiyas negiyas marked this pull request as draft September 8, 2023 03:08
@negiyas negiyas marked this pull request as ready for review September 8, 2023 05:34
@negiyas negiyas changed the title [WIP] Fix constant propagation pass issue for onnx.Gather op. Fix constant propagation pass issue for onnx.Gather op. Sep 8, 2023
@sorenlassen
Copy link
Member

actually, I think the real problem is that the const prop implementation only works for int64 indices and not int32, sorry about that

rank 0 indices seems to work fine for int64

please see the alternative fix in PR #2488

@negiyas
Copy link
Collaborator Author

negiyas commented Sep 8, 2023

@sorenlassen Your comments are right. I am closing this PR. Thank you for your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants