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

Fix xnnpack pybind #2802

Closed
wants to merge 3 commits into from
Closed

Fix xnnpack pybind #2802

wants to merge 3 commits into from

Conversation

mcr229
Copy link
Contributor

@mcr229 mcr229 commented Apr 1, 2024

Fixing XNNPACK Pybind.

Currently pybindings for xnnpack fail due to uninitialized error. We notice the following:

(gdb) info symbol xnn_initialize
xnn_initialize in section .text of /home/maxren/.conda/envs/executorch/lib/python3.10/site-packages/torch/lib/libtorch_cpu.so
(gdb) info symbol xnn_create_subgraph
xnn_create_subgraph in section .text of /home/maxren/.conda/envs/executorch/lib/python3.10/site-packages/executorch/extension/pybindings/portable_lib.cpython-310-x86_64-linux-gnu.so

That is xnn_initialize symbole comes from libtorch_cpu, but when we call xnn_create_subgraph we call it from another library. It seems like explicitly adding XNNPACK to the portable_libs pybind fixes this problem.

Additionaly, we fix some fPIC issues when building XNNPACK.

Copy link

pytorch-bot bot commented Apr 1, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2802

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 25e55f2 with merge base 15d9ddd (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 1, 2024
@mcr229 mcr229 force-pushed the fix_xnnpack_pybind branch from 83e5e23 to bd4e4ef Compare April 1, 2024 23:49
Copy link
Contributor

@metascroy metascroy left a comment

Choose a reason for hiding this comment

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

LGTM...you tested this fixes the pybind xnnpack issue?

@facebook-github-bot
Copy link
Contributor

@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: As titled. Add executorch/backends/xnnpack/test/models
directory into pytest.ini

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link
Contributor

@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mcr229 merged this pull request in 26d7bcc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants