Skip to content

[LLVM][CMake][MSVC] Wrap linker options for ICX with LLVM_BUILD_INSTUMENTED #124573

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

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

Maetveis
Copy link
Contributor

RFC: https://discourse.llvm.org/t/rfc-cmake-linker-flags-need-wl-equivalent-for-intel-c-icx-on-windows/82446

The Intel C++ Compiler (ICX) passes linker flags through the driver unlike MSVC and clang-cl, and therefore needs them to be prefixed with /Qoption,link (the equivalent of -Wl, for gcc on *nix).

Previous PRs did not catch this because I did not try building with LLVM_BUILD_INSTRUMENTED=ON.
CMAKE_CXX_LINKER_WRAPPER_FLAG is empty for plain clang-cl (icx on windows behaves as clang-cl), so this is NFC for clang-cl.

The approach I used to find this is documented as github gist here: https://gist.github.com/Maetveis/00567488f0d6ff74095d91ed306fafc5

…UMENTED

RFC: https://discourse.llvm.org/t/rfc-cmake-linker-flags-need-wl-equivalent-for-intel-c-icx-on-windows/82446

The Intel C++ Compiler (ICX) passes linker flags through the driver
unlike MSVC and clang-cl, and therefore needs them to be prefixed with
`/Qoption,link` (the equivalent of `-Wl,` for gcc on *nix).

Previous PRs did not catch this because I did not try building with
`LLVM_BUILD_INSTRUMENTED=ON`.
`CMAKE_CXX_LINKER_WRAPPER_FLAG` is empty for plain clang-cl
(icx on windows behaves as clang-cl), so this is NFC for clang-cl.

The approach I used to find these is documented as github gist here:
https://gist.github.com/Maetveis/00567488f0d6ff74095d91ed306fafc5
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Jan 27, 2025
Copy link
Contributor

@tcreech-intel tcreech-intel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Maetveis Maetveis merged commit e1b5826 into llvm:main Jan 28, 2025
10 checks passed
@Maetveis Maetveis deleted the wrap_linker_flag_instrumented branch January 28, 2025 12:49
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 29, 2025

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2189

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/38/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-12688-38-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=38 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants