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.

Sign up for GitHub

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

Jump to bottom

[popsift] Workaround cuda unsupported _MSC_VER is 1940 #36146

Closed
wants to merge 3 commits into from
Closed

[popsift] Workaround cuda unsupported _MSC_VER is 1940 #36146

wants to merge 3 commits into from

Conversation

Cheney-W
Copy link
Contributor

@Cheney-W Cheney-W commented Jan 12, 2024 •
edited
Loading

popsift:x64-windows failed in an internal version of Visual Studio 2022 which the _MSC_VER is 1940:

C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v12.1\include\crt/host_config.h(153): fatal error C1189: #error: -- unsupported Microsoft Visual Studio version! Only the versions between 2017 and 2022 (inclusive) are supported! The nvcc flag '-allow-unsupported-compiler' can be used to override this version check; however, using an unsupported host compiler may cause compilation failure or incorrect run time execution.  Use at your own risk.
  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@Cheney-W Cheney-W added category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. labels Jan 12, 2024
@Neumann-A
Copy link
Contributor

Neumann-A commented Jan 12, 2024 •
edited
Loading

INVALID (The correct fix involves adding -allow-unsupported-compiler into the toolchain for CMAKE_CUDA_FLAGS instead of fixing single ports)

Neumann-A
Neumann-A reviewed Jan 18, 2024
View reviewed changes
scripts/toolchains/windows.cmake
@@ -72,6 +72,8 @@ if(NOT _CMAKE_IN_TRY_COMPILE)
set(CMAKE_C_FLAGS_DEBUG "/D_DEBUG ${VCPKG_CRT_LINK_FLAG_PREFIX}d /Z7 /Ob0 /Od /RTC1 ${VCPKG_C_FLAGS_DEBUG}" CACHE STRING "")
set(CMAKE_CXX_FLAGS_RELEASE "${VCPKG_CRT_LINK_FLAG_PREFIX} /O2 /Oi /Gy /DNDEBUG /Z7 ${VCPKG_CXX_FLAGS_RELEASE}" CACHE STRING "")
set(CMAKE_C_FLAGS_RELEASE "${VCPKG_CRT_LINK_FLAG_PREFIX} /O2 /Oi /Gy /DNDEBUG /Z7 ${VCPKG_C_FLAGS_RELEASE}" CACHE STRING "")
# Workaround cuda unsupported _MSC_VER is 1940
set(CUDA_NVCC_FLAGS "-allow-unsupported-compiler" CACHE STRING "")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also required for CMAKE_CUDA_FLAGS. But shouldn't be in the main repo as long as the internal VS version does not officially exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried adding '-allow-unsupported-compiler' to CMAKE_CUDA_FLAGS here, but it didn't work.
Also, I attempted to use CMAKE_CXX_COMPILER_VERSION as a control condition, but its value is empty in this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I attempted to use CMAKE_CXX_COMPILER_VERSION as a control condition, but its value is empty in this file.

That is because compiler detection runs after the toolchain not before.

I have tried adding '-allow-unsupported-compiler' to CMAKE_CUDA_FLAGS here, but it didn't work.

you probably need to have both CUDA_NVCC_FLAGS and CMAKE_CUDA_FLAGS. The former is for the deprecated way to use cuda via FindCUDA while the latter is used for the case of enable_language(CUDA) or via the LANGUAGES defined in project()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your guidance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, do you have any suggestions on how to determine the compiler version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Within vcpkg $ENV{VSCMD_VER} can probably be used since vcpkg runs within a dev prompt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion. I tested it locally, but $ENV{VSCMD_VER} can only display Visual Studio versions like 17.8.4, and cannot show the specific version of cl.exe, so it still cannot be used as a condition for judgment.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should be using a custom toolchain file when testing the compiler that does this instead

@Cheney-W Cheney-W closed this Jul 15, 2024
@Cheney-W
Copy link
Contributor Author

Issue solved by PR #39325,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BillyONeal BillyONeal BillyONeal left review comments

@Neumann-A Neumann-A Neumann-A left review comments

Assignees

@LilyWangLL LilyWangLL

Labels
category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Cheney-W @Neumann-A @BillyONeal @LilyWangLL