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

Update FusePackIntoLoop -> FuseProducerIntoLoop to include linalg.copy op #1050

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

yzhang93
Copy link
Contributor

In cases that linalg.copy ops are used instead of tensor.pack ops as the producers of the computation op, they should be able to be fused into the loops in the same way. This PR updates the pass to handle such cases.

@newling
Copy link
Contributor

newling commented Jan 22, 2025

This looks good to me, but an alternative option would be to factorize this into a new pass iree-amdaie-copy-to-pack, run that after after any pass that might introduce linalg.copy (ire-amdaie-pad etc), and then we can always assume there are no linalg.copy

@yzhang93
Copy link
Contributor Author

This looks good to me, but an alternative option would be to factorize this into a new pass iree-amdaie-copy-to-pack, run that after after any pass that might introduce linalg.copy (ire-amdaie-pad etc), and then we can always assume there are no linalg.copy

I know this trick would work as we used it as an intermediate step in ConvertToDmaPass, but I'm not sure if it's a good idea to write it as a formal pass at tensor level. In the definition of PackOp, the original tensor of rank n is packed into a result tensor of rank n + k where k>0 as shown here https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td#L1887

@newling
Copy link
Contributor

newling commented Jan 22, 2025

:-0 so if those docs are correct you can have an identity IREE::LinalgExt::PackOp but not an identity Linalg::PackOp...

Copy link
Contributor

@Abhishek-Varma Abhishek-Varma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Abhishek-Varma Abhishek-Varma merged commit 3a6aa4a into nod-ai:main Jan 23, 2025
7 checks passed
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.

3 participants