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

[clang-repl][CMake][MSVC] Wrap /EXPORT linker option for ICX #112867

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

Maetveis
Copy link
Contributor

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).

Use the LINKER: prefix for the /EXPORT: options in clang-repl, this
expands to the correct flag for ICX and nothing for MSVC / clang-cl.

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

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Maetveis and the rest of your teammates on Graphite Graphite

@Maetveis Maetveis marked this pull request as ready for review October 18, 2024 09:40
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/pr-subscribers-clang

Author: Mészáros Gergely (Maetveis)

Changes

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).

Use the LINKER: prefix for the /EXPORT: options in clang-repl, this
expands to the correct flag for ICX and nothing for MSVC / clang-cl.

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


Full diff: https://github.com/llvm/llvm-project/pull/112867.diff

2 Files Affected:

  • (modified) clang/tools/clang-repl/CMakeLists.txt (+2-4)
  • (modified) clang/unittests/Interpreter/CMakeLists.txt (+2-5)
diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt
index 9ffe853d759caf..7aebbe7a19436a 100644
--- a/clang/tools/clang-repl/CMakeLists.txt
+++ b/clang/tools/clang-repl/CMakeLists.txt
@@ -48,11 +48,9 @@ if(MSVC)
   endif()
 
   # List to '/EXPORT:sym0 /EXPORT:sym1 /EXPORT:sym2 ...'
-  foreach(sym ${clang_repl_exports})
-    set(clang_repl_link_str "${clang_repl_link_str} /EXPORT:${sym}")
-  endforeach(sym ${clang_repl_exports})
+  list(TRANSFORM clang_repl_exports PREPEND "LINKER:/EXPORT:")
 
-  set_property(TARGET clang-repl APPEND_STRING PROPERTY LINK_FLAGS ${clang_repl_link_str})
+  set_property(TARGET clang-repl APPEND PROPERTY LINK_OPTIONS ${clang_repl_exports})
 
 endif(MSVC)
 
diff --git a/clang/unittests/Interpreter/CMakeLists.txt b/clang/unittests/Interpreter/CMakeLists.txt
index 1ed1216c772e8f..95378f9cfe7370 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -67,10 +67,7 @@ if(MSVC)
   endif()
 
   # List to '/EXPORT:sym0 /EXPORT:sym1 /EXPORT:sym2 ...'
-  foreach(sym ${ClangReplInterpreterTests_exports})
-    set(ClangReplInterpreterTests_link_str "${ClangReplInterpreterTests_link_str} /EXPORT:${sym}")
-  endforeach(sym ${ClangReplInterpreterTests_exports})
-
-  set_property(TARGET ClangReplInterpreterTests APPEND_STRING PROPERTY LINK_FLAGS ${ClangReplInterpreterTests_link_str})
+  list(TRANSFORM ClangReplInterpreterTests_exports PREPEND "LINKER:/EXPORT:")
+  set_property(TARGET ClangReplInterpreterTests APPEND PROPERTY LINK_OPTIONS ${ClangReplInterpreterTests_exports})
 
 endif(MSVC)

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).

Use the `LINKER:` prefix for the `/EXPORT:` options in clang-repl, this
expands to the correct flag for ICX and nothing for MSVC / clang-cl.

RFC: https://discourse.llvm.org/t/rfc-cmake-linker-flags-need-wl-equivalent-for-intel-c-icx-on-windows/82446
@Maetveis Maetveis force-pushed the users/maetveis/wrap-icx-clang branch from 7e73f86 to 2c2b407 Compare October 23, 2024 11:04
@Maetveis Maetveis merged commit eb9af19 into main Oct 23, 2024
6 of 8 checks passed
@Maetveis Maetveis deleted the users/maetveis/wrap-icx-clang branch October 23, 2024 12:33
@frobtech frobtech mentioned this pull request Oct 25, 2024
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 2, 2024

LLVM Buildbot has detected a new failure on builder clang-s390x-linux-multistage running on systemz-1 while building clang at step 11 "ninja check 2".

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

Here is the relevant piece of the build log for the reference
Step 11 (ninja check 2) failure: stage 2 checked (failure)
******************** TEST 'libFuzzer-s390x-default-Linux :: fuzzer-timeout.test' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/./bin/clang    -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/lib/fuzzer  /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/TimeoutTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest
+ /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/./bin/clang -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/lib/fuzzer /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/TimeoutTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest
RUN: at line 2: /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/./bin/clang    -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/lib/fuzzer  /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/TimeoutEmptyTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutEmptyTest
+ /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/./bin/clang -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/lib/fuzzer /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/TimeoutEmptyTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutEmptyTest
RUN: at line 3: not  /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 2>&1 | FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=TimeoutTest
+ not /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1
+ FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=TimeoutTest
RUN: at line 12: not  /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/hi.txt 2>&1 | FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=SingleInputTimeoutTest
+ not /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/hi.txt
+ FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=SingleInputTimeoutTest
RUN: at line 16: /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 -timeout_exitcode=0
+ /home/uweigand/sandbox/buildbot/clang-s390x-linux-multistage/stage2/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 -timeout_exitcode=0
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3451328230
INFO: Loaded 1 modules   (13 inline 8-bit counters): 13 [0x2aa0eccfe60, 0x2aa0eccfe6d), 
INFO: Loaded 1 PC tables (13 PCs): 13 [0x2aa0eccfe70,0x2aa0eccff40), 
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#2	INITED cov: 2 ft: 2 corp: 1/1b exec/s: 0 rss: 31Mb
#788	NEW    cov: 3 ft: 3 corp: 2/3b lim: 11 exec/s: 0 rss: 31Mb L: 2/2 MS: 1 InsertByte-
#834	NEW    cov: 4 ft: 4 corp: 3/4b lim: 11 exec/s: 0 rss: 31Mb L: 1/2 MS: 1 EraseBytes-
#5230	NEW    cov: 5 ft: 5 corp: 4/7b lim: 53 exec/s: 0 rss: 32Mb L: 3/3 MS: 1 InsertByte-
#5611	NEW    cov: 6 ft: 6 corp: 5/9b lim: 53 exec/s: 0 rss: 32Mb L: 2/3 MS: 1 CrossOver-
ALARM: working on the last Unit for 1 seconds
       and the timeout value is 1 (use -timeout=N to change)
MS: 1 InsertRepeatedBytes-; base unit: 0c1bc52c50016933679b0980ccff3680e5831162
0x48,0x69,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0xa,
Hi!!!!!!!!!!!!!\012
artifact_prefix='./'; Test unit written to ./timeout-cb93ec1f37348dd693be56166acc79f801c74ceb
Base64: SGkhISEhISEhISEhISEhCg==
==2977106== ERROR: libFuzzer: timeout after 1 seconds
AddressSanitizer:DEADLYSIGNAL
=================================================================
AddressSanitizer:DEADLYSIGNAL
=================================================================
AddressSanitizer: CHECK failed: asan_report.cpp:199 "((current_error_.kind)) == ((kErrorKindInvalid))" (0x1, 0x0) (tid=2977106)
    <empty stack>

MS: 1 InsertRepeatedBytes-; base unit: 0c1bc52c50016933679b0980ccff3680e5831162
0x48,0x69,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0x21,0xa,
Hi!!!!!!!!!!!!!\012
artifact_prefix='./'; Test unit written to ./crash-cb93ec1f37348dd693be56166acc79f801c74ceb
Base64: SGkhISEhISEhISEhISEhCg==

--
...

NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…2867)

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).

Use the `LINKER:` prefix for the `/EXPORT:` options in clang-repl, this
expands to the correct flag for ICX and nothing for MSVC / clang-cl.

RFC:
https://discourse.llvm.org/t/rfc-cmake-linker-flags-need-wl-equivalent-for-intel-c-icx-on-windows/82446
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants