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

Support TSL compilation on Windows with CUDA support. #15499

Closed

Conversation

eaplatanios
Copy link
Contributor

This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

@@ -36,7 +36,7 @@ namespace {
// nvtxNameOsThreadA:
// https://nvidia.github.io/NVTX/doxygen/group___r_e_s_o_u_r_c_e___n_a_m_i_n_g.html
// This convention may not match the one in tsl::Env::GetCurrentThreadId().
std::optional<uint32_t> GetCurrentThreadId() {
std::optional<uint32_t> MaybeGetCurrentThreadId() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoids a name conflict.

configured_version = "%{cuda_version}"
configured_major = int(configured_version.split('.')[0])
configured_minor = int(configured_version.split('.')[1])
# Strip "64_" which appears in the CUDA version on Windows.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly self-explanatory. In Windows, the CUDA version looks something like 64_121. There are other parts of the build that already handle such version numbers but it was not being handled properly here.

@@ -181,6 +181,9 @@ def InvokeNvcc(argv, log=False):
nvccopts += ['--keep', '--keep-dir', tempdir]
# Force C++17 dialect (note, everything in just one string!)
nvccopts += ['--std c++17']
# This is so that nvcc does not complain about MSVC or CLANG.
nvccopts += ['-allow-unsupported-compiler']
nvccopts += ['--expt-extended-lambda', '--expt-relaxed-constexpr']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary but also mirrors settings that are also used in crosstool_wrapper_driver_is_not_gcc.

@ddunl
Copy link
Member

ddunl commented Jul 30, 2024

LGTM, this one will require a patch internally I think so I'll try to get to that soon. Thanks!!

@eaplatanios
Copy link
Contributor Author

Great, thanks @ddunl!

@NaiyerRizz NaiyerRizz self-assigned this Jul 31, 2024
copybara-service bot pushed a commit that referenced this pull request Jul 31, 2024
Imported from GitHub PR #15444

This PR fixes some issues I bumped into when trying to compile XLA on Windows. I still haven't gotten GPU support to work but I'm making progress. The CPU only version compiles fine after some of the changes in this PR. I'll point out some specific issues this PR fixes in comments.

There are also TSL-specific changes that are pulled in a separate PR (#15499).
Copybara import of the project:

--
eacee95 by eaplatanios <e.a.platanios@gmail.com>:

Fixed some issues around compiling on Windows.

--
b12e4cf by eaplatanios <e.a.platanios@gmail.com>:

.

--
e23ef17 by eaplatanios <e.a.platanios@gmail.com>:

.

--
bdae19b by eaplatanios <e.a.platanios@gmail.com>:

.

--
2f90e6b by eaplatanios <e.a.platanios@gmail.com>:

.

--
5700979 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a978b1f by eaplatanios <e.a.platanios@gmail.com>:

.

--
d7fe81d by eaplatanios <e.a.platanios@gmail.com>:

.

--
fc40d91 by eaplatanios <e.a.platanios@gmail.com>:

.

--
326aec3 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a7603b7 by eaplatanios <e.a.platanios@gmail.com>:

.

--
edcc97a by eaplatanios <e.a.platanios@gmail.com>:

.

--
cec2448 by eaplatanios <e.a.platanios@gmail.com>:

.

--
df3eb22 by eaplatanios <e.a.platanios@gmail.com>:

.

--
8997345 by eaplatanios <e.a.platanios@gmail.com>:

.

--
219a9f1 by eaplatanios <e.a.platanios@gmail.com>:

.

--
73f3cd7 by eaplatanios <e.a.platanios@gmail.com>:

.

Merging this change closes #15444

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15444 from eaplatanios:u/eaplatanios/cpp-17-fixes 73f3cd7
PiperOrigin-RevId: 657913961
copybara-service bot pushed a commit that referenced this pull request Jul 31, 2024
Imported from GitHub PR #15444

This PR fixes some issues I bumped into when trying to compile XLA on Windows. I still haven't gotten GPU support to work but I'm making progress. The CPU only version compiles fine after some of the changes in this PR. I'll point out some specific issues this PR fixes in comments.

There are also TSL-specific changes that are pulled in a separate PR (#15499).
Copybara import of the project:

--
eacee95 by eaplatanios <e.a.platanios@gmail.com>:

Fixed some issues around compiling on Windows.

--
b12e4cf by eaplatanios <e.a.platanios@gmail.com>:

.

--
e23ef17 by eaplatanios <e.a.platanios@gmail.com>:

.

--
bdae19b by eaplatanios <e.a.platanios@gmail.com>:

.

--
2f90e6b by eaplatanios <e.a.platanios@gmail.com>:

.

--
5700979 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a978b1f by eaplatanios <e.a.platanios@gmail.com>:

.

--
d7fe81d by eaplatanios <e.a.platanios@gmail.com>:

.

--
fc40d91 by eaplatanios <e.a.platanios@gmail.com>:

.

--
326aec3 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a7603b7 by eaplatanios <e.a.platanios@gmail.com>:

.

--
edcc97a by eaplatanios <e.a.platanios@gmail.com>:

.

--
cec2448 by eaplatanios <e.a.platanios@gmail.com>:

.

--
df3eb22 by eaplatanios <e.a.platanios@gmail.com>:

.

--
8997345 by eaplatanios <e.a.platanios@gmail.com>:

.

--
219a9f1 by eaplatanios <e.a.platanios@gmail.com>:

.

--
73f3cd7 by eaplatanios <e.a.platanios@gmail.com>:

.

Merging this change closes #15444

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15444 from eaplatanios:u/eaplatanios/cpp-17-fixes 73f3cd7
PiperOrigin-RevId: 657913961
copybara-service bot pushed a commit that referenced this pull request Jul 31, 2024
Imported from GitHub PR #15444

This PR fixes some issues I bumped into when trying to compile XLA on Windows. I still haven't gotten GPU support to work but I'm making progress. The CPU only version compiles fine after some of the changes in this PR. I'll point out some specific issues this PR fixes in comments.

There are also TSL-specific changes that are pulled in a separate PR (#15499).
Copybara import of the project:

--
eacee95 by eaplatanios <e.a.platanios@gmail.com>:

Fixed some issues around compiling on Windows.

--
b12e4cf by eaplatanios <e.a.platanios@gmail.com>:

.

--
e23ef17 by eaplatanios <e.a.platanios@gmail.com>:

.

--
bdae19b by eaplatanios <e.a.platanios@gmail.com>:

.

--
2f90e6b by eaplatanios <e.a.platanios@gmail.com>:

.

--
5700979 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a978b1f by eaplatanios <e.a.platanios@gmail.com>:

.

--
d7fe81d by eaplatanios <e.a.platanios@gmail.com>:

.

--
fc40d91 by eaplatanios <e.a.platanios@gmail.com>:

.

--
326aec3 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a7603b7 by eaplatanios <e.a.platanios@gmail.com>:

.

--
edcc97a by eaplatanios <e.a.platanios@gmail.com>:

.

--
cec2448 by eaplatanios <e.a.platanios@gmail.com>:

.

--
df3eb22 by eaplatanios <e.a.platanios@gmail.com>:

.

--
8997345 by eaplatanios <e.a.platanios@gmail.com>:

.

--
219a9f1 by eaplatanios <e.a.platanios@gmail.com>:

.

--
73f3cd7 by eaplatanios <e.a.platanios@gmail.com>:

.

Merging this change closes #15444

FUTURE_COPYBARA_INTEGRATE_REVIEW=#15444 from eaplatanios:u/eaplatanios/cpp-17-fixes 73f3cd7
PiperOrigin-RevId: 657913961
copybara-service bot pushed a commit that referenced this pull request Jul 31, 2024
Imported from GitHub PR #15444

This PR fixes some issues I bumped into when trying to compile XLA on Windows. I still haven't gotten GPU support to work but I'm making progress. The CPU only version compiles fine after some of the changes in this PR. I'll point out some specific issues this PR fixes in comments.

There are also TSL-specific changes that are pulled in a separate PR (#15499).
Copybara import of the project:

--
eacee95 by eaplatanios <e.a.platanios@gmail.com>:

Fixed some issues around compiling on Windows.

--
b12e4cf by eaplatanios <e.a.platanios@gmail.com>:

.

--
e23ef17 by eaplatanios <e.a.platanios@gmail.com>:

.

--
bdae19b by eaplatanios <e.a.platanios@gmail.com>:

.

--
2f90e6b by eaplatanios <e.a.platanios@gmail.com>:

.

--
5700979 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a978b1f by eaplatanios <e.a.platanios@gmail.com>:

.

--
d7fe81d by eaplatanios <e.a.platanios@gmail.com>:

.

--
fc40d91 by eaplatanios <e.a.platanios@gmail.com>:

.

--
326aec3 by eaplatanios <e.a.platanios@gmail.com>:

.

--
a7603b7 by eaplatanios <e.a.platanios@gmail.com>:

.

--
edcc97a by eaplatanios <e.a.platanios@gmail.com>:

.

--
cec2448 by eaplatanios <e.a.platanios@gmail.com>:

.

--
df3eb22 by eaplatanios <e.a.platanios@gmail.com>:

.

--
8997345 by eaplatanios <e.a.platanios@gmail.com>:

.

--
219a9f1 by eaplatanios <e.a.platanios@gmail.com>:

.

--
73f3cd7 by eaplatanios <e.a.platanios@gmail.com>:

.

Merging this change closes #15444

COPYBARA_INTEGRATE_REVIEW=#15444 from eaplatanios:u/eaplatanios/cpp-17-fixes 73f3cd7
PiperOrigin-RevId: 657937707
@sgerrard sgerrard requested a review from ddunl August 12, 2024 22:40
copybara-service bot pushed a commit to google/tsl that referenced this pull request Aug 12, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit that referenced this pull request Aug 12, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes #15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Aug 12, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit that referenced this pull request Aug 13, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes #15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Aug 13, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to google/tsl that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes #15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to google/tsl that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes #15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 662267938
copybara-service bot pushed a commit to google/tsl that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 663079395
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Aug 14, 2024
This is following up on #15444. There's still one issue blocking Windows support that I haven't resolved. I've described it here. Any suggestions/advice on how to proceed for that one would be helpful. cc @hawkinsp @ddunl and also fyi @metab0t.

This closes openxla/xla#15499.

PiperOrigin-RevId: 663079395
@vam-google
Copy link
Contributor

How is this checking that CUDA works on Windows? Is it compilation-specific? If it compiles on windows it does not mean it works on windows

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.

4 participants