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

[Feature Request] DPS version of logical_xor op #13758

Closed
Tracked by #13795
mmanzoorTT opened this issue Oct 11, 2024 · 9 comments
Closed
Tracked by #13795

[Feature Request] DPS version of logical_xor op #13758

mmanzoorTT opened this issue Oct 11, 2024 · 9 comments
Assignees
Labels

Comments

@mmanzoorTT
Copy link

TTNN lib provides DPS based op for logical_and, logical_or, and logical_not. However, logical_xor is implemented as non-DPS op. Can we convert logical_xor to DPS op as well to be consistent?

@sdjordjevicTT
Copy link
Contributor

Hi @eyonland, we discovered this during our onboarding in the TT-MLIR compiler. Can you give us some feedback on whether this inconsistency in the logical_xor op is a limitation or just needs to be implemented from your side?

@eyonland eyonland assigned umadevimcw and unassigned eyonland Oct 18, 2024
@eyonland
Copy link
Contributor

We need to investigate this a bit more to be sure, but I don't see why we can't get this implemented for you.

@sdjordjevicTT
Copy link
Contributor

Thanks @eyonland for the response! Let's keep this issue up to date with the progress of the implementation.

@umadevimcw
Copy link
Contributor

@mmanzoorTT @sdjordjevicTT Updated the logical XOR similar to other logical ops like AND, OR, in this PR #14340. Let us know if this meets your requirements

@umadevimcw
Copy link
Contributor

@sdjordjevicTT @mmanzoorTT Merged the PR. Kindly check and let me know if we can close this issue.

@mmanzoorTT
Copy link
Author

@umadevimcw Thanks a lot. I'll test it today and will comment. PR looks good overall.

@eyonland
Copy link
Contributor

eyonland commented Nov 2, 2024

@mmanzoorTT , can we close this issue?

@umadevimcw
Copy link
Contributor

Closing this issue as the PR is merged to main. Will reopen if needed

@mmanzoorTT
Copy link
Author

@umadevimcw @eyonland @sdjordjevicTT I have implemented/tested DPS logical_xor op with tenstorrent/tt-mlir#1141 (requiring tenstorrent/tt-mlir#1119 for tt-metal uplift) and it is working as expected. thanks a lot

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

No branches or pull requests

5 participants