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

Legalize sqrt to tosa #18

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

ttjost
Copy link

@ttjost ttjost commented May 22, 2023

No description provided.

@ttjost ttjost requested a review from mgehre-amd May 22, 2023 08:22
@mgehre-amd
Copy link
Collaborator

There is already an end2end test for sqrt (look for ElementwiseSqrtModule). Can you add it to the TOSA_PASS set in e2e_testing/xfail_sets.py?
You can then run it locally via ./tools/e2e_test.sh --config=tosa -v -f ElementwiseSqrtModule

Then you also don't need the IR tests. torch-mlir uses end2end tests for almost everything, and will reject PRs that add IR unit tests when they can also be covered by end2end.

Copy link
Collaborator

@mgehre-amd mgehre-amd left a comment

Choose a reason for hiding this comment

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

LGTM!

  • Please do the test changes

@ttjost
Copy link
Author

ttjost commented May 22, 2023

There is already an end2end test for sqrt (look for ElementwiseSqrtModule). Can you add it to the TOSA_PASS set in e2e_testing/xfail_sets.py? You can then run it locally via ./tools/e2e_test.sh --config=tosa -v -f ElementwiseSqrtModule

Then you also don't need the IR tests. torch-mlir uses end2end tests for almost everything, and will reject PRs that add IR unit tests when they can also be covered by end2end.

Did not know that. Thanks for letting me know. I'll update PR.

@ttjost ttjost force-pushed the tiagot.legalize_sqrt_to_tosa branch from 1acbed2 to 5e066c1 Compare May 22, 2023 09:51
@mgehre-amd mgehre-amd merged commit c12f76a into misc_fixes May 22, 2023
@mgehre-amd mgehre-amd mentioned this pull request Jun 5, 2023
51 tasks
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