intel / torch-xpu-ops

Apache License 2.0
30 stars 21 forks source link

[Windows] Add MSVC Flag #946

Closed ratnampa closed 1 month ago

ratnampa commented 1 month ago

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

fengyuan14 commented 1 month ago

Is it a duplicate PR same as #945?

fengyuan14 commented 1 month ago

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

fengyuan14 commented 1 month ago

Is it a duplicate PR same as #945?

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

ratnampa commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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.

tye1 commented 1 month ago

@ratnampa Let me close this PR, as already merged in https://github.com/intel/torch-xpu-ops/commit/02060c616661034286adb9d5ba493e234b8401c3.