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

[ONNX] Remove kernel shape and weight shape equivalence check from Onnx.Conv lowering #3869

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

vivekkhandelwal1
Copy link
Collaborator

@vivekkhandelwal1 vivekkhandelwal1 commented Nov 13, 2024

This commit removes the equivalence check for kernel shape and weight shape from the Onnx.conv lowering since those checks seem to be of no use (not sure why were they part of the lowering in the first place).

Signed-Off By: Vivek Khandelwal [email protected]

This commit adds the support for handling dynamically shaped weight
tensor by adding runtime checks for comparing the weight shape and kernel
shape.

Signed-Off By: Vivek Khandelwal <[email protected]>
@vivekkhandelwal1 vivekkhandelwal1 marked this pull request as ready for review November 13, 2024 10:14
@zjgarvey
Copy link
Collaborator

So I'm a bit curious about a few things:

  1. Why do we need to check if the provided kernel shape matches in the first place? The kernel shape is ... what the kernel shape is?
  2. What models did this come up in? What motivated this patch?

@vivekkhandelwal1 vivekkhandelwal1 changed the title [ONNX] Handle dynamic shape weight in Onnx.Conv lowering [ONNX] Remove kernel shape and weight shape equivalence check from Onnx.Conv lowering Nov 14, 2024
@vivekkhandelwal1
Copy link
Collaborator Author

Hi @zjgarvey, the PR is updated. Can you please review it now?

@vivekkhandelwal1 vivekkhandelwal1 merged commit fe2f649 into llvm:main Nov 15, 2024
3 checks passed
@vivekkhandelwal1 vivekkhandelwal1 deleted the fix-onnx-conv branch November 15, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants