Skip to content

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Feb 7, 2025

I previously moved it to the wrong place because of build issues. Had to couple this with replacing sizes_to_string so that the buck2->cmake system would place it in executorch_core instead of portable_kernels and not break the build.

[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 Feb 7, 2025

Copy link

pytorch-bot bot commented Feb 7, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure, 7 Pending

As of commit f98f69f with merge base 77f18b2 (image):

NEW FAILURE - The following job has failed:

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 Feb 7, 2025
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: 5c2cfff
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
@swolchok
Copy link
Contributor Author

swolchok commented Feb 7, 2025

this doesn't build because the new file is getting placed in portable_kernels__srcs by the buck2->cmake stuff, but kernels/portable/CMakeLists.txt doesn't respect portable_kernels__srcs and instead globs it itself! long-term plan is to stop using buck2 to drive CMake builds, so a short-term plan of "couple this with replacing dim_order_util's sizes_to_string" seems OK.

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: d86b665
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: c98fe3d
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: 8696dcf
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: d17c223
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: 86ec3ad
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
@swolchok swolchok requested review from GregoryComer, dbort and kirklandsign and removed request for GregoryComer and dbort February 7, 2025 18:02
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Feb 7, 2025
I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: 0b97f31
ghstack-comment-id: 2641470188
Pull Request resolved: #8296
Base automatically changed from gh/swolchok/204/head to main February 7, 2025 19:12
@swolchok swolchok merged commit 883d33a into main Feb 7, 2025
107 of 111 checks passed
@swolchok swolchok deleted the gh/swolchok/235/head branch February 7, 2025 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk 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