Skip to content

Properly export ET_USE_THREADPOOL from the threadpool extension #8947

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

Merged
merged 28 commits into from
Mar 7, 2025

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Mar 4, 2025

We had a bunch of targets that would define this macro ad-hoc. It's supposed to indicate that the threadpool extension is available, so just make sure that we have it as a PUBLIC target_compile_definition in CMake and an exported_preprocessor_flags entry in Buck for extension_threadpool.

Test Plan: CI on following NOCOMMIT PR, which fails builds if ET_USE_THREADPOOL is not defined in affected places.

Needed for #8932.

swolchok added 5 commits March 4, 2025 11:35
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
Copy link

pytorch-bot bot commented Mar 4, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit 5a283c8 with merge base 484231c (image):
💚 Looks good so far! There are no failures yet. 💚

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

swolchok added 10 commits March 4, 2025 18:23
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@swolchok
Copy link
Contributor Author

swolchok commented Mar 5, 2025

noting that CI is 100% green in case I rebase

swolchok added 2 commits March 5, 2025 12:41
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Mar 5, 2025
We had a bunch of targets that would define this macro ad-hoc. It's supposed to indicate that the threadpool extension is available, so just make sure that we have it as a PUBLIC `target_compile_definition` in CMake and an `exported_preprocessor_flags` entry in Buck for extension_threadpool.

Test Plan: CI on following NOCOMMIT PR, which fails builds if ET_USE_THREADPOOL is not defined in affected places.

ghstack-source-id: 9b3a0ca
ghstack-comment-id: 2699165786
Pull Request resolved: #8947
swolchok added 4 commits March 5, 2025 19:37
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@swolchok swolchok changed the base branch from gh/swolchok/307/head to gh/swolchok/318/head March 6, 2025 17:56
swolchok added 3 commits March 7, 2025 09:48
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Mar 7, 2025
…s/lla{m,v}a/CMakeLists.txt (#8946)

The old way was a holdover from when custom_ops lived in examples/models/llama (and was copy/pasted to llava and Android).

Needed for #8947.
Base automatically changed from gh/swolchok/318/head to main March 7, 2025 20:37
@swolchok swolchok merged commit f23971a into main Mar 7, 2025
51 checks passed
@swolchok swolchok deleted the gh/swolchok/308/head branch March 7, 2025 20:38
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. topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants