8000 [cpp_extension][inductor] Fix sleef windows depends. by xuhancn · Pull Request #128770 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

[cpp_extension][inductor] Fix sleef windows depends. #128770

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

Closed
wants to merge 1 commit into from

Conversation

xuhancn
Copy link
Collaborator
@xuhancn xuhancn commented Jun 15, 2024

Issue:

During I'm working on enable inductor on PyTorch Windows, I found the sleef lib dependency issue.
image

Analysis:

After we enabled SIMD on PyTorch Windows(#118980 ), the sleef functions are called from VEC headers. It bring the sleef to the dependency.

Here is a different between Windows and Linux OS.

Linux :

Linux is default export its functions, so libtorch_cpu.so static link to sleef.a, and then It also export sleef's functions.
image

Windows:

Windows is by default not export its functions, and have many limitation to export functions, reference: #80604
We can't package sleef functions via torch_cpu.dll like Linux.

Solution:

Acturally, we also packaged sleef static lib as a part of release. We just need to help user link to sleef.lib, it should be fine.

  1. Add sleef to cpp_builder for inductor.
  2. Add sleef to cpp_extension for C++ extesion.

cc @peterjc123 @mszhanyi @skyline75489 @nbcsm @vladimir-aubrecht @iremyux @Blackhex @cristianPanaite @malfet @zou3519 @jbschlosser @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @voznesenskym @penguinwu @EikanWang @Guobing-Chen @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

Copy link
pytorch-bot bot commented Jun 15, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 90c9027 with merge base b50c0e9 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@xuhancn xuhancn added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 15, 2024
@xuhancn
Copy link
Collaborator Author
xuhancn commented Jun 15, 2024

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased xu_fix_windows_sleef_depends onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout xu_fix_windows_sleef_depends && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the xu_fix_windows_sleef_depends branch from a25d456 to 90c9027 Compare June 15, 2024 08:50
@xuhancn xuhancn added module: windows Windows support for PyTorch module: cpp-extensions Related to torch.utils.cpp_extension intel This tag is for PR from Intel module: cpp Related to C++ API ciflow/binaries_libtorch Trigger binary build and upload jobs for libtorch on the PR labels Jun 15, 2024
@xuhancn xuhancn marked this pull request as ready for review June 15, 2024 11:08
@xuhancn xuhancn requested a review from jgong5 June 15, 2024 11:09
@xuhancn xuhancn requested a review from jansel June 17, 2024 02:17
@xuhancn xuhancn changed the title fix sleef windows depends. [cpp_extension] Fix sleef windows depends. Jun 17, 2024
@xuhancn xuhancn changed the title [cpp_extension] Fix sleef windows depends. [cpp_extension][inductor] Fix sleef windows depends. Jun 17, 2024
@jansel
Copy link
Contributor
jansel commented Jun 17, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@jgong5 jgong5 added the topic: not user facing topic category label Jun 17, 2024
@jgong5
Copy link
Collaborator
jgong5 commented Jun 17, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

xuhancn added a commit to xuhancn/pytorch that referenced this pull request Jun 17, 2024
# Issue:
During I'm working on enable inductor on PyTorch Windows, I found the sleef lib dependency issue.
<img width="1011" alt="image" src="https://github.com/pytorch/pytorch/assets/8433590/423bd854-3c5f-468f-9a64-a392d9b514e3">

# Analysis:
After we enabled SIMD on PyTorch Windows(pytorch#118980 ), the sleef functions are called from VEC headers. It bring the sleef to the dependency.

Here is a different between Windows and Linux OS.
## Linux :
Linux is default export its functions, so libtorch_cpu.so static link to sleef.a, and then It also export sleef's functions.
<img width="647" alt="image" src="https://github.com/pytorch/pytorch/assets/8433590/00ac536c-33fc-4943-a435-25590508840d">

## Windows:
Windows is by default not export its functions, and have many limitation to export functions, reference: pytorch#80604
We can't package sleef functions via torch_cpu.dll like Linux.

# Solution:
Acturally, we also packaged sleef static lib as a part of release. We just need to help user link to sleef.lib, it should be fine.
1. Add sleef to cpp_builder for inductor.
2. Add sleef to cpp_extension for C++ extesion.

Pull Request resolved: pytorch#128770
Approved by: https://github.com/jgong5, https://github.com/jansel
pytorchmergebot pushed a commit to xuhancn/pytorch that referenced this pull request Jun 21, 2024
# Issue:
During I'm working on enable inductor on PyTorch Windows, I found th
8000
e sleef lib dependency issue.
<img width="1011" alt="image" src="https://github.com/pytorch/pytorch/assets/8433590/423bd854-3c5f-468f-9a64-a392d9b514e3">

# Analysis:
After we enabled SIMD on PyTorch Windows(pytorch#118980 ), the sleef functions are called from VEC headers. It bring the sleef to the dependency.

Here is a different between Windows and Linux OS.
## Linux :
Linux is default export its functions, so libtorch_cpu.so static link to sleef.a, and then It also export sleef's functions.
<img width="647" alt="image" src="https://github.com/pytorch/pytorch/assets/8433590/00ac536c-33fc-4943-a435-25590508840d">

## Windows:
Windows is by default not export its functions, and have many limitation to export functions, reference: pytorch#80604
We can't package sleef functions via torch_cpu.dll like Linux.

# Solution:
Acturally, we also packaged sleef static lib as a part of release. We just need to help user link to sleef.lib, it should be fine.
1. Add sleef to cpp_builder for inductor.
2. Add sleef to cpp_extension for C++ extesion.

Pull Request resolved: pytorch#128770
Approved by: https://github.com/jgong5, https://github.com/jansel
atalman pushed a commit that referenced this pull request Jun 26, 2024
# Issue:
During I'm working on enable inductor on PyTorch Windows, I found the sleef lib dependency issue.
<img width="1011" alt="image" src="https://github.com/pytorch/pytorch/assets/8433590/423bd854-3c5f-468f-9a64-a392d9b514e3">

# Analysis:
After we enabled SIMD on PyTorch Windows(#118980 ), the sleef functions are called from VEC headers. It bring the sleef to the dependency.

Here is a different between Windows and Linux OS.
## Linux :
Linux is default export its functions, so libtorch_cpu.so static link to sleef.a, and then It also export sleef's functions.
<img width="647" alt="image" src="https://github.com/pytorch/pytorch/assets/8433590/00ac536c-33fc-4943-a435-25590508840d">

## Windows:
Windows is by default not export its functions, and have many limitation to export functions, reference: #80604
We can't package sleef functions via torch_cpu.dll like Linux.

# Solution:
Acturally, we also packaged sleef static lib as a part of release. We just need to help user link to sleef.lib, it should be fine.
1. Add sleef to cpp_builder for inductor.
2. Add sleef to cpp_extension for C++ extesion.

Pull Request resolved: #128770
Approved by: https://github.com/jgong5, https://github.com/jansel
alinpahontu2912 added a commit to iremyux/pytorch that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/binaries_libtorch Trigger binary build and upload jobs for libtorch on the PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request intel This tag is for PR from Intel Merged module: cpp Related to C++ API module: cpp-extensions Related to torch.utils.cpp_extension module: inductor module: windows Windows support for PyTorch open source topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0