Skip to content

Fix setup.py to skip CPU kernels on Windows#3187

Merged
Xia-Weiwen merged 6 commits into
pytorch:mainfrom
Xia-Weiwen:fix_build_win
Nov 5, 2025
Merged

Fix setup.py to skip CPU kernels on Windows#3187
Xia-Weiwen merged 6 commits into
pytorch:mainfrom
Xia-Weiwen:fix_build_win

Conversation

@Xia-Weiwen

@Xia-Weiwen Xia-Weiwen commented Oct 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Fixes #3175

CPU kernel files in the torchao/csrc/cpu/aten_kernels/ directory are not skipped correctly on Windows because the path string matching fails (\ vs /). This PR fixes this issue.

Test plan
We have verified this patch in our local environment (Processor: 12th Gen Intel(R) Core(TM) i7-1270P, OS: Windows 11 Enterprise).

@pytorch-bot

pytorch-bot Bot commented Oct 16, 2025 •
edited
Loading

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/3187

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

✅ No Failures

As of commit 8036d0e with merge base 9266734 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@meta-cla meta-cla 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 Oct 16, 2025
@Xia-Weiwen Xia-Weiwen added module: not user facing Use this tag if you don't want this PR to show up in release notes and removed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Oct 16, 2025
@Xia-Weiwen Xia-Weiwen requested a review from vkuzo October 16, 2025 05:08
@Xia-Weiwen Xia-Weiwen marked this pull request as ready for review October 16, 2025 05:08
@Xia-Weiwen Xia-Weiwen mentioned this pull request Oct 16, 2025
Closed
@meta-cla meta-cla 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 Oct 16, 2025
@Xia-Weiwen

Xia-Weiwen commented Oct 17, 2025

Copy link
Copy Markdown
Collaborator Author

CI is broken. Failures are not related to this PR.

@Xia-Weiwen

Xia-Weiwen commented Oct 17, 2025

Copy link
Copy Markdown
Collaborator Author

@mingfeima Please review. Thanks.

@Xia-Weiwen Xia-Weiwen requested a review from jerryzh168 October 17, 2025 01:17
@Xia-Weiwen

Xia-Weiwen commented Oct 20, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo @jerryzh168 @metascroy @mingfeima @Valentine233 Please review this PR. Thanks.

@Xia-Weiwen

Xia-Weiwen commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo @jerryzh168 @metascroy Can you please review this PR? Thanks.

1 similar comment
@Xia-Weiwen

Xia-Weiwen commented Oct 23, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo @jerryzh168 @metascroy Can you please review this PR? Thanks.

@vkuzo

vkuzo commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

could you add a test plan detailing how this was tested for correctness? Thank you!

@Xia-Weiwen

Xia-Weiwen commented Oct 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

could you add a test plan detailing how this was tested for correctness? Thank you!

Hi @vkuzo If there is a CI workflow that builds Torchao on Windows, this will be tested. Do you have a plan to enable that in CI? Thanks.

@Xia-Weiwen Xia-Weiwen mentioned this pull request Oct 24, 2025
Closed
@Xia-Weiwen

Xia-Weiwen commented Oct 24, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo I have submitted an issue here #3237 Can we add such a workflow in CI to test it? Thanks.

@Xia-Weiwen

Xia-Weiwen commented Oct 27, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo How do you think about adding Windows building in CI? Or do you have more suggestions? Thanks.

@Xia-Weiwen

Xia-Weiwen commented Oct 28, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @metascroy @vkuzo @jerryzh168 @cpuhrsch @atalman @msaroufim @janeyx99 @drisspg Could you please review this PR? Thanks.

@vkuzo

vkuzo commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

yeah, I think having XPU in CI would help with more confidence on PRs like this one, it's not ideal to rely on other people testing the changes without a clear way to reproduce them

@Xia-Weiwen

Xia-Weiwen commented Oct 28, 2025

Copy link
Copy Markdown
Collaborator Author

yeah, I think having XPU in CI would help with more confidence on PRs like this one, it's not ideal to rely on other people testing the changes without a clear way to reproduce them

The change is not about XPU but Windows. Do you mean Windows in CI? Do you have any suggestions on adding a workflow to test building on Windows? Thanks.

@Xia-Weiwen Xia-Weiwen mentioned this pull request Oct 29, 2025
Merged
@Xia-Weiwen

Xia-Weiwen commented Oct 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo I understand setting up a workflow for Windows build in CI is not a simple task for you. We have verified this patch on our local machine. I have also update the summary above. Do you think we can land it now? Thanks.

@Xia-Weiwen

Xia-Weiwen commented Oct 30, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @vkuzo Would you mind if we land this? This fix is needed to resolve #3175 . Without this fix, building on Windows will still fail even with 30082cb . And we have verified the fix in our local environment. Thanks.

@Xia-Weiwen

Xia-Weiwen commented Oct 31, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @metascroy @vkuzo @jerryzh168 @cpuhrsch @atalman @msaroufim @janeyx99 @drisspg do you think we can land this now? Thanks.

@Xia-Weiwen

Xia-Weiwen commented Nov 3, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @metascroy @vkuzo @jerryzh168 @cpuhrsch @atalman @msaroufim @janeyx99 @drisspg Would you mind if we land this now? Thanks.

@Xia-Weiwen

Xia-Weiwen commented Nov 3, 2025

Copy link
Copy Markdown
Collaborator Author

Hi @metascroy @vkuzo @jerryzh168 @cpuhrsch @atalman @msaroufim @janeyx99 @drisspg Would you mind if we land this now? Thanks.

It's a bug in setup.py and I guess you want to fix it too.

@pytorch-bot

pytorch-bot Bot commented Nov 4, 2025

Copy link
Copy Markdown

Warning: Unknown label ciflow/binaries/all.
Currently recognized labels are

  • ciflow/benchmark
  • ciflow/tutorials
  • ciflow/rocm
  • ciflow/4xh100
  • ciflow/xpu

Please add the new label to .github/pytorch-probot.yml

@Xia-Weiwen Xia-Weiwen merged commit 1c89061 into pytorch:main Nov 5, 2025
37 of 38 checks passed
@Xia-Weiwen Xia-Weiwen deleted the fix_build_win branch November 5, 2025 07:15
namgyu-youn pushed a commit to namgyu-youn/ao that referenced this pull request Nov 21, 2025
@Xia-Weiwen @namgyu-youn
Fix setup.py to skip CPU kernels on Windows (pytorch#3187)
6e1de50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Valentine233 Valentine233 Valentine233 approved these changes

@vkuzo vkuzo Awaiting requested review from vkuzo vkuzo is a code owner

@metascroy metascroy Awaiting requested review from metascroy

@jerryzh168 jerryzh168 Awaiting requested review from jerryzh168 jerryzh168 is a code owner

@cpuhrsch cpuhrsch Awaiting requested review from cpuhrsch

@msaroufim msaroufim Awaiting requested review from msaroufim

@janeyx99 janeyx99 Awaiting requested review from janeyx99

@drisspg drisspg Awaiting requested review from drisspg

+2 more reviewers

@atalman atalman atalman approved these changes

@mingfeima mingfeima mingfeima approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ciflow/binaries/all CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: not user facing Use this tag if you don't want this PR to show up in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Windows][build]it will meet build failures when build with pytorch xpu version on windows

5 participants

@Xia-Weiwen @vkuzo @atalman @mingfeima @Valentine233