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

[Windows] Add MSVC Flag #946

Open
wants to merge 1 commit into
base: release/2.5
Choose a base branch
from

Conversation

ratnampa
Copy link

Adding /permissive- flag for Copy.cpp compilation failed with IPEX 2.5

@fengyuan14
Copy link
Contributor

Is it a duplicate PR same as #945?

@fengyuan14
Copy link
Contributor

Hi @ratnampa, most likely, we would not cherry-pick the commit to PyTorch release/2.5.

@fengyuan14
Copy link
Contributor

Is it a duplicate PR same as #945?

I see, it is a commit to release/2.5 branch.

@ratnampa
Copy link
Author

ratnampa commented Sep 30, 2024

@fengyuan14 we won't need to cherry pick to PyTorch release/2.5 as the upstream 2.5 build works fine. But for private-pytorch 2.5, which is used by IPEX-2.5, build on windows it fails with ambiguous symbol 'std' error due to MSVC issue. This flag adds stricter conformance to compilation, and it is able to resolve such errors.

@tye1
Copy link

tye1 commented Oct 8, 2024

@fengyuan14 @ratnampa we need fix it for private PyTorch 2.5. Do we need create a new branch in 'torch-xpu-ops' if 'release/2.5' is used for stock PyTorch only?

@tye1
Copy link

tye1 commented Oct 8, 2024

@fengyuan14 we won't need to cherry pick to PyTorch release/2.5 as the upstream 2.5 build works fine. But for private-pytorch 2.5, which is used by IPEX-2.5, build on windows it fails with ambiguous symbol 'std' error due to MSVC issue. This flag adds stricter conformance to compilation, and it is able to resolve such errors.

This is not correct, upstream 2.5 build will fail also if it is built with oneAPI 2024.2.1 or oneAPI 2025.0, right? We won't fix it because upstream 2.5 does not support oneAPI newer version. For upstream 2.6, we need resolve it as it need support oneAPI 2025.0.

@ratnampa
Copy link
Author

ratnampa commented Oct 8, 2024

Yes, so with the oneAPI bundle it doesn't fail for Upstream 2.5 so no need for cherry-pick. For PT 2.6 we will need to add the fix when using updated oneAPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants