Skip to content

Commit 019edfe

Browse files
committed
[SYCL][E2E] Stop CMAKE_CXX_FLAGS cache variable passing to clang++
Before #9075 the value of CMAKE_CXX_FLAGS was passed to SYCL E2E compilations. In #9075 the variable was explicitly unset in non-standalone (in-tree) builds of the tests. Unsetting a normal variable however exposes the cmake cache variable of the same name. I believe this was not the intent, rather the intent was to not pass any CMAKE_CXX_FLAGS to the E2E tests in non-standalone builds. It is weird to pass the user's CMAKE_CXX_FLAGS to the E2E tests, as these flags are for the SYCL toolchain, not tests. Passing `CMAKE_CXX_FLAGS` to the E2E tests can cause problems, for example a test might wish to override options such as setting `-ffp-model=fast`, but this can fails with a warning/error if `CMAKE_CXX_FLAGS` contains `-ffp-model=precise`: `error: overriding '-ffp-model=precise' option with '-ffp-model=fast' [-Werror,-Woverriding-option]`. While the error could be worked around in the test by appending `-Wno-overriding-option` to the test flags, I don't believe this is the right solution. It is possible that the user is running on a system where some flags have to be passed to clang++ to make it produce working executables. For this reason, allow setting `SYCL_E2E_CLANG_CXX_FLAGS` by the user if needed in in-tree builds, but do not pass `CMAKE_CXX_FLAGS` by default. Contains a drive-by fix to make sure `-Werror` is passed to clang++ even when the c++ flags are overriden by the lit parameter `--param cxx_flags=...` (it can still be disabled using `-Wno-error` explicitly).
1 parent cb03a1b commit 019edfe

File tree

2 files changed

+4
-22
lines changed

2 files changed

+4
-22
lines changed

sycl/test-e2e/CMakeLists.txt

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,11 @@ endif() # Standalone.
3939

4040
if(SYCL_TEST_E2E_STANDALONE)
4141
set(SYCL_CXX_COMPILER ${CMAKE_CXX_COMPILER})
42+
set(SYCL_E2E_CLANG_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
4243
else()
4344
set(SYCL_CXX_COMPILER "${LLVM_BINARY_DIR}/bin/clang++")
44-
# Don't want options used for building sycl-toolchain.
45-
unset(CMAKE_CXX_FLAGS)
45+
set(SYCL_E2E_CLANG_CXX_FLAGS "" CACHE STRING
46+
"Flags passed to clang++ when building SYCL end-to-end tests")
4647
endif() # Standalone.
4748

4849
find_package(Threads REQUIRED)
@@ -65,25 +66,6 @@ if(NOT SYCL_TEST_E2E_TARGETS)
6566
set(SYCL_TEST_E2E_TARGETS "all")
6667
endif()
6768

68-
if(MSVC AND NOT SYCL_TEST_E2E_STANDALONE)
69-
# We're trying to pass MSVC flags to Clang, which doesn't work by default
70-
separate_arguments(cxx_flags NATIVE_COMMAND "${CMAKE_CXX_FLAGS}")
71-
foreach(flag IN ITEMS ${cxx_flags})
72-
# Skip certain flags that only exists for MSVC
73-
if("${flag}" STREQUAL "/EHsc")
74-
continue()
75-
endif()
76-
# Change the way compiler definitions are passed in
77-
string(REGEX REPLACE "^/D" "-D" clang_flag "${flag}")
78-
list(APPEND SYCL_E2E_CLANG_CXX_FLAGS ${clang_flag})
79-
endforeach()
80-
string (REPLACE ";" " " SYCL_E2E_CLANG_CXX_FLAGS "${SYCL_E2E_CLANG_CXX_FLAGS}")
81-
else()
82-
set(SYCL_E2E_CLANG_CXX_FLAGS ${CMAKE_CXX_FLAGS})
83-
endif()
84-
85-
set(SYCL_E2E_CLANG_CXX_FLAGS "${SYCL_E2E_CLANG_CXX_FLAGS} -Werror")
86-
8769
if(NOT DEFINED CUDA_LIBS_DIR AND NOT DEFINED CUDA_INCLUDE)
8870
find_package(CUDAToolkit)
8971

sycl/test-e2e/lit.cfg.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1147,7 +1147,7 @@ def get_sycl_ls_verbose(sycl_device, env):
11471147
config.substitutions.append(("%clangxx", " true "))
11481148
config.substitutions.append(("%clang", " true "))
11491149
else:
1150-
clangxx = " " + config.dpcpp_compiler + " "
1150+
clangxx = " " + config.dpcpp_compiler + " -Werror "
11511151
if "preview-mode" in config.available_features:
11521152
# Technically, `-fpreview-breaking-changes` is reported as unused option
11531153
# if used without `-fsycl`. However, we have far less tests compiling

0 commit comments

Comments
 (0)