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 build fix #2738

Closed
wants to merge 9 commits into from
Closed

Windows build fix #2738

wants to merge 9 commits into from

Conversation

wkpark
Copy link
Contributor

@wkpark wkpark commented Dec 1, 2023

based on #2465

  • manually applied, rebased, fix lint errors
  • use set_target_properties(), cleanup for windows
  • remove '/A' platform option to use windows ninja, remove unknown option '/m'
  • use sysconfig.get_config_var() to get the path of python*.lib
  • use os.name to check dll extension
  • clang fix for windows
  • remove '-fPIC' for windows clang

support both clang and msvc

See also (with workflows fixes)

@ptillet
Copy link
Collaborator

ptillet commented Dec 2, 2023

I think python/triton/common/build.py is becoming way too complicated. It would be great to have abstractions instead of another if/else statement

@wkpark wkpark force-pushed the windows-fix branch 2 times, most recently from 85916b4 to edbfb14 Compare December 2, 2023 12:58
@wkpark
Copy link
Contributor Author

wkpark commented Dec 2, 2023

I think python/triton/common/build.py is becoming way too complicated. It would be great to have abstractions instead of another if/else statement

I don't think it's gotten too complicated, but I've tried to clean up _build() a bit.

@wkpark
Copy link
Contributor Author

wkpark commented Dec 3, 2023

rebased, partially squashed

@wkpark wkpark force-pushed the windows-fix branch 2 times, most recently from becd537 to 6b3c9c6 Compare December 3, 2023 11:00
@wkpark
Copy link
Contributor Author

wkpark commented Dec 3, 2023

now runtime tests are passed with minor fixes

  • test_subproc.py needs "spawn"
  • test_cache.py with tempfile.NamedTemporaryFile(mode='w+', suffix='.py', delete=False)
(venv) D:\src\triton\python\test\unit>python -m pytest runtime
======================================================= test session starts =======================================================
platform win32 -- Python 3.10.11, pytest-7.4.2, pluggy-1.3.0
rootdir: D:\src\triton\python
plugins: anyio-3.7.1, hydra-core-1.3.2, xdist-3.5.0
collected 21 items

runtime\test_autotuner.py ....                                                                                               [ 19%]
runtime\test_cache.py .............                                                                                          [ 80%]
runtime\test_driver.py .                                                                                                     [ 85%]
runtime\test_launch.py .                                                                                                     [ 90%]
runtime\test_subproc.py ..                                                                                                   [100%]

======================================================= 21 passed in 25.69s =======================================================

If you prefer to clean up commits to reduce the number, I'll do a partial squash, so let me know.

@wkpark wkpark force-pushed the windows-fix branch 2 times, most recently from 5290766 to 7147517 Compare December 4, 2023 08:41
@FurkanGozukara
Copy link

thank you so much
so currently any pip install link for python 3.10.x?

@wkpark
Copy link
Contributor Author

wkpark commented Dec 5, 2023

thank you so much so currently any pip install link for python 3.10.x?

a direct pip link is not available yet but downloadable artifacts are available (working on next workflows PR)
https://github.com/wkpark/triton/actions/runs/7091492250 (github login required. scroll down and you can find downloadable links)

@wkpark
Copy link
Contributor Author

wkpark commented Dec 5, 2023

rebased

@FurkanGozukara
Copy link

thank you so much so currently any pip install link for python 3.10.x?

a direct pip link is not available yet but downloadable artifacts are available (working on next workflows PR) https://github.com/wkpark/triton/actions/runs/7091492250 (github login required. scroll down and you can find downloadable links)

thank you so much. i am waiting direct pip link hopefully

@wkpark
Copy link
Contributor Author

wkpark commented Dec 8, 2023

rebased, conflict resolved.

@FurkanGozukara
Copy link

amazing still waiting direct pip install

@FurkanGozukara
Copy link

awesome more fix. any pip install link so far?

@wkpark
Copy link
Contributor Author

wkpark commented Dec 17, 2023

just rebased, resolve conflicts

@wkpark
Copy link
Contributor Author

wkpark commented Dec 17, 2023

https://github.com/wkpark/triton/actions/runs/7097090430
https://github.com/wkpark/triton/actions/runs/7081762873

If anyone interested in this PR
follow the above link, download the wheel file at the bottom of the page (github login required), install the wheel on your Windows system, and leave your results here to help the developers. (using pip install command...)

If you have a little more time on your hands, you might want to try running a pytest in the directory under python/test/unit after cloning the triton repo., and report back here with your results.

@techcto
Copy link

techcto commented Dec 18, 2023

https://github.com/wkpark/triton/actions/runs/7097090430

If anyone interested in this PR follow the above link, download the wheel file at the bottom of the page (github login required), install the wheel on your Windows system, and leave your results here to help the developers. (using pip install command...)

If you have a little more time on your hands, you might want to try running a pytest in the directory under python/test/unit after cloning the triton repo., and report back here with your results.

Is there a Windows Python 311 to test?

@FurkanGozukara
Copy link

https://github.com/wkpark/triton/actions/runs/7097090430

If anyone interested in this PR follow the above link, download the wheel file at the bottom of the page (github login required), install the wheel on your Windows system, and leave your results here to help the developers. (using pip install command...)

If you have a little more time on your hands, you might want to try running a pytest in the directory under python/test/unit after cloning the triton repo., and report back here with your results.

it is only python 3.9 i think is that accurate?

@wkpark wkpark force-pushed the windows-fix branch 2 times, most recently from 2c5a8ee to 20d41c4 Compare January 23, 2024 09:07
@wkpark
Copy link
Contributor Author

wkpark commented Jan 23, 2024

rebased with minor fix.
https://github.com/wkpark/triton/actions/runs/7623880319 (latest triton builds)

@FurkanGozukara
Copy link

rebased with minor fix. https://github.com/wkpark/triton/actions/runs/7623880319 (latest triton builds)

awesome ty

@wkpark
Copy link
Contributor Author

wkpark commented Feb 10, 2024

rebased, conflict resolved.
build wheels will be available soon.

https://github.com/wkpark/triton/actions/runs/7855606361

how to test?

  1. install the prebuilt wheel from https://github.com/wkpark/triton/actions/runs/7855606361 (github login required. you can find Windows wheel at the bottom of the page)
  2. clone triton repo: https://github.com/openai/triton
  3. open visual studio 2022 cmd prompt.
  4. check cl command. set CC and CXX env. (set CC=cl and set CXX=cl in the cmd prompt)
  5. cd python\test\unit
  6. python -m pytest -s runtime

see also #2738 (comment)

 * based on triton-lang#2465
 * manually applied, rebased, fix lint errors
 * use set_target_properties(), cleanup for windows
 * remove '/A' platform option to use windows ninja
 * remove unknown option '/m'
 * use sysconfig.get_config_var() to get the path of python*.lib
 * clang fix for windows
 * remove '-fPIC' for windows clang
 * fix download_and_copy() to support windows
 * add "exe" extension for windows
 * use "pyd" extension for windows to make importlib work
 * rework for latest triton (2024/01/14)

Original-author-by: Andrei Gheorghe <[email protected]>
Signed-off-by: Won-Kyu Park <[email protected]>
 * based on Windows support PR triton-lang#2456 by @andreigh
 * WIN32 fix using LoadLibrary
clang error
"(aka 'long long') must match previous return type 'long' when lambda expression has unspecified explicit return typ"
 * win32 fix _path_to_binary()
 * add library_dir, include_dir for win32
@FurkanGozukara
Copy link

@wkpark any new link of downloading newest push pre compiled wheel?

@ptillet
Copy link
Collaborator

ptillet commented Mar 9, 2024

Sorry, but I will unfortunately close this. I don't think we have the bandwidth to commit to supporting windows upstream. Maybe this can be done in a fork for now?

@ptillet ptillet closed this Mar 9, 2024
@biship
Copy link

biship commented Mar 9, 2024

Yeah no one uses Windows.

@FurkanGozukara
Copy link

Windows is most commonly used OS what are you dreaming?

@dgoryeo
Copy link

dgoryeo commented Mar 10, 2024

Haha, I think it was meant to be sarcastic :)

What is mind-boggling to me is that openai does not support the OS platform of their largest investor that has given them $13 billion so far. I really wonder ...

@FurkanGozukara
Copy link

Haha, I think it was meant to be sarcastic :)

What is mind-boggling to me is that openai does not support the OS platform of their largest investor that has given them $13 billion so far. I really wonder ...

100%

I complained about this too

@wkpark
Copy link
Contributor Author

wkpark commented Mar 10, 2024

Sorry, but I will unfortunately close this. I don't think we have the bandwidth to commit to supporting windows upstream. Maybe this can be done in a fork for now?

no problem, I can understand. that ways opensource goes. but I'm not consider any fork. instead I will try to extract some part of mergeable hunks, for example, clang for windows work perfectly. we don't have to support MSVC, instead we can make clang compatible code for both windows and linux in mind. etc.
thank you for this great project and thank you for your concern!

@showkeyjar
Copy link

@wkpark how to use your "llvm-4017f04e-windows-x64" release? compile it?

@FurkanGozukara
Copy link

please return back publishing @wkpark

we need you : THUDM/CogVLM2#169

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.

9 participants