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

[nvtt] Control CUDA dependency, fix linkage #24888

Merged
merged 3 commits into from
May 25, 2022
Merged

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented May 24, 2022

  • What does your PR fix?

    Fixes a warning about the unused debug postfix in the release build.
    Fixes wrong CRT linkage for MSVC release builds.
    Adds an opt-in feature which controls the CUDA dependency.
    Fixes an installation order problem detected in a vcpkg CI build of port osg when port cuda wasn't installed, but needed by the cached nvtt package.
    Other changes to port nvtt are out of scope for this PR.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    unchanged, no.

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@dg0yt
Copy link
Contributor Author

dg0yt commented May 24, 2022

Note on CI results: The binary artifact for port nvtt was already cached from build of #24720.

@dg0yt dg0yt marked this pull request as draft May 24, 2022 06:11
@Adela0814 Adela0814 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label May 24, 2022
@dg0yt
Copy link
Contributor Author

dg0yt commented May 24, 2022

Now examining another osg (port update) build issue in release build only, x64-windows:

nvtt.lib(CompressionOptions.cpp.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in NVTTImageProcessor.cpp.obj

and similar.

Looking at the logs from the forced nvtt build failure.
x64-windows debug uses /MDd:

[56/106] C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe   /TP -DNVTT_EXPORTS -D__MMX__ -D__SSE2__ -D__SSE__ -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\src -ID:\buildtrees\nvtt\x64-windows-dbg\src -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\extern\poshlib -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\extern\stb -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\src\nvtt -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\extern\rg_etc1_v104 /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /GR /EHsc /MP  /D_DEBUG /MDd /Z7 /Ob0 /Od /RTC1  -std:c++14 /showIncludes /Fosrc\nvtt\CMakeFiles\nvtt.dir\CompressionOptions.cpp.obj /Fdsrc\nvtt\CMakeFiles\nvtt.dir\nvtt.pdb /FS -c D:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\src\nvtt\CompressionOptions.cpp

x64-windows release uses no flag but probably should use /MD?

[60/106] C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe   /TP -DNVTT_EXPORTS -D__MMX__ -D__SSE2__ -D__SSE__ -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\src -ID:\buildtrees\nvtt\x64-windows-rel\src -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\extern\poshlib -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\extern\stb -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\src\nvtt -ID:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\extern\rg_etc1_v104 /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /GR /EHsc /MP   /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /GR /EHsc /MP  /O2 /Ob2 /Oi /Ot /Oy /GL -std:c++14 /showIncludes /Fosrc\nvtt\CMakeFiles\nvtt.dir\CompressionOptions.cpp.obj /Fdsrc\nvtt\CMakeFiles\nvtt.dir\nvtt.pdb /FS -c D:\buildtrees\nvtt\src\2188752e3f-17c078710b.clean\src\nvtt\CompressionOptions.cpp

I see, it doesn't save the existing CMAKE_<lang>_FLAGS_RELEASE in cmake/OptimalOptions.cmake.
(I wonder why it only fails in my port osg update.)

@dg0yt
Copy link
Contributor Author

dg0yt commented May 24, 2022

Now there is /MD for release:

[59/106] C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe   /TP -DNVTT_EXPORTS -D__MMX__ -D__SSE2__ -D__SSE__ -ID:\buildtrees\nvtt\src\2188752e3f-686fd8e62c.clean\src -ID:\buildtrees\nvtt\x64-windows-rel\src -ID:\buildtrees\nvtt\src\2188752e3f-686fd8e62c.clean\extern\poshlib -ID:\buildtrees\nvtt\src\2188752e3f-686fd8e62c.clean\extern\stb -ID:\buildtrees\nvtt\src\2188752e3f-686fd8e62c.clean\src\nvtt -ID:\buildtrees\nvtt\src\2188752e3f-686fd8e62c.clean\extern\rg_etc1_v104 /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /GR /EHsc /MP  /MD /O2 /Oi /Gy /DNDEBUG /Z7  /O2 /Ob2 /Oi /Ot /Oy /GL -std:c++14 /showIncludes /Fosrc\nvtt\CMakeFiles\nvtt.dir\CompressionOptions.cpp.obj /Fdsrc\nvtt\CMakeFiles\nvtt.dir\nvtt.pdb /FS -c D:\buildtrees\nvtt\src\2188752e3f-686fd8e62c.clean\src\nvtt\CompressionOptions.cpp

@dg0yt dg0yt marked this pull request as ready for review May 24, 2022 06:51
@dg0yt dg0yt changed the title [nvtt] Control CUDA dependency [nvtt] Control CUDA dependency, fix linkage May 24, 2022
@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label May 25, 2022
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Wow, that's a lot of bugs. Thanks!

@ras0219-msft ras0219-msft merged commit ee0d840 into microsoft:master May 25, 2022
@dg0yt
Copy link
Contributor Author

dg0yt commented May 26, 2022

@ras0219-msft Is it intentional that check_crt_linkage_of_libs is called for Build::LinkageType::STATIC, but not for Build::LinkageType::DYNAMIC in https://github.com/microsoft/vcpkg-tool/blob/main/src/vcpkg/postbuildlint.cpp#L1074-L1129?
I feel like the inconsistent linkage should have been discovered by the postbuild checks.

@dg0yt dg0yt deleted the nvtt branch May 26, 2022 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants