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

Add e2e support for aten logical or/and/xor/not ops #1752

Merged
merged 1 commit into from
Dec 26, 2022

Conversation

li-plus
Copy link
Collaborator

@li-plus li-plus commented Dec 23, 2022

This PR supported lowering AtenLogicalOrOp to MHLO, and added AtenLogicalAndOp, AtenLogicalXorOp and AtenLogicalNotOp for both LINALG and MHLO backends.

Copy link
Collaborator

@tanyokwok tanyokwok left a comment

Choose a reason for hiding this comment

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

LGTM

@tanyokwok tanyokwok merged commit eaab9be into llvm:main Dec 26, 2022
Comment on lines +1433 to +1435
INSERT_BINARY_LOGICAL_PATTERN(AtenLogicalOrOp, chlo::BroadcastOrOp);
INSERT_BINARY_LOGICAL_PATTERN(AtenLogicalAndOp, chlo::BroadcastAndOp);
INSERT_BINARY_LOGICAL_PATTERN(AtenLogicalXorOp, chlo::BroadcastXorOp);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason these changes don't pass the e2e tests?

@ashay
Copy link
Collaborator

ashay commented Dec 30, 2022

@li-plus The E2E tests introduced in this commit seem to be non-deterministically failing in the post-merge build. Here is an issue to track the problem and PR #1757 reverts this change so that subsequent PRs are unblocked. Could you look into the failure please? Thanks!

@li-plus
Copy link
Collaborator Author

li-plus commented Dec 30, 2022

@ashay Okay, I have reproduced the error locally and submitted a fix in #1761.

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.

4 participants