[SYCL] Handle massive warnings comming from DPCPP lib #1583

fadeeval · 2020-04-24T13:17:10Z

Handle massive warnings comming from DPCPP lib.

Signed-off-by: Aleksander Fadeev aleksander.fadeev@intel.com

sycl/include/CL/sycl/intel/sub_group_host.hpp

sycl/include/CL/sycl/program.hpp

bader

Please, don't use hard coded paths.

bader · 2020-04-24T14:49:36Z

sycl/test/warnings/warnings.cpp

@@ -1,4 +1,4 @@
-// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
+// RUN: %clangxx -I /localdisk2/icl/fadeeval/sycl_workspace/build/include/sycl -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version  %s -o %t.out


Suggested change

// RUN: %clangxx -I /localdisk2/icl/fadeeval/sycl_workspace/build/include/sycl -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version %s -o %t.out

// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version %s -o %t.out

romanovvlad

Ok overall. But what is dpcxx lib?

romanovvlad · 2020-04-27T09:21:18Z

sycl/include/CL/__spirv/spirv_vars.hpp

-  template <> size_t get##POSTFIX<0>() { return __spirv_##POSTFIX##_x(); }     \
-  template <> size_t get##POSTFIX<1>() { return __spirv_##POSTFIX##_y(); }     \
-  template <> size_t get##POSTFIX<2>() { return __spirv_##POSTFIX##_z(); }
+  template <> inline size_t get##POSTFIX<0>() { return __spirv_##POSTFIX##_x(); }     \


Please, clang-format the patch.

romanovvlad · 2020-04-27T09:25:47Z

sycl/test/warnings/warnings.cpp

@@ -1,4 +1,4 @@
-// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
+// RUN: %clangxx -I /localdisk2/icl/fadeeval/sycl_workspace/build/include/sycl -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version  %s -o %t.out


What about "-pedantic" ? Should we add this option?

What is "-pedantic"?

s-kanaev

I believe these __UNUSED_ON_SYCL_DEVICE and __UNUSED_ON_HOST should have some prefix to show these are from SYCL RT.
Also, is it possible to change these macro's to non-function-like?

bader · 2020-04-28T09:49:02Z

I believe these __UNUSED_ON_SYCL_DEVICE and __UNUSED_ON_HOST should have some prefix to show these are from SYCL RT.
Also, is it possible to change these macro's to non-function-like?

I believe macro definitions should be avoided if possible and I think it's possible.

sycl/include/CL/sycl/detail/defines.hpp

bader · 2020-04-30T12:17:36Z

sycl/include/CL/sycl/ordered_queue.hpp

@@ -263,7 +263,16 @@ class __SYCL_EXPORT_DEPRECATED("Replaced by in_order queue property")

 namespace std {
 template <> struct hash<cl::sycl::ordered_queue> {


@jbrodman, are you okay if we just remove this header file?

bader · 2020-04-30T12:20:35Z

sycl/test/CMakeLists.txt

@@ -6,6 +6,7 @@ set(SYCL_INCLUDE "${SYCL_INCLUDE_BUILD_DIR}")
 set(SYCL_TOOLS_SRC_DIR "${PROJECT_SOURCE_DIR}/tools/")
 set(LLVM_BUILD_BINARY_DIRS "${LLVM_BINARY_DIR}/bin/")
 set(LLVM_BUILD_LIBRARY_DIRS "${LLVM_BINARY_DIR}/lib/")
+set(SOURCE_SYCL_INCLUDE "${CMAKE_CURRENT_SOURCE_DIR}/../include")


There is already sycl_inc_dir varible for that.

bader · 2020-04-30T12:21:36Z

sycl/test/warnings/warnings.cpp

@@ -1,4 +1,4 @@
-// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
+// RUN: %clangxx -fsycl -I %sycl_source_libs_dir -Wall -Wextra -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version  %s -o %t.out


Why not sycl_include?

sycl_include point to CL/sycl.hpp in the build directory, but I want that the path points to CL/sycl.hpp in the llvm directory. Otherwise -fsycl -I didn't work.

But the name that I gave is bad, I wanted to name it "%source_sycl_include", but mixed up. I will change it.

@romanovvlad

Dismissing "change requested" for @romanovvlad.

bader · 2020-05-21T09:59:50Z

llvm/test/Bindings/Go/go.test

@@ -2,4 +2,3 @@

 ; REQUIRES: shell
 ; UNSUPPORTED: asan, ubsan, msan


Please, remove all unrelated changes from your patch.

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Werror is added Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Path fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> __SYCL_UNUSED_ARUMENT__ Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> __SYCL_UNUSED_ARGUMENT__ Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> path fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> usm_allocator.hpp fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> test setup Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> warnings.cpp path correction Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Formatting Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> suppres depricate warning Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Path creating Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> warnings.cpp fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> defines.hpp revert Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> ordered_queue.cpp fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Path improvement Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Formatting 3 Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> pragma fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> FIX rebase Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Formatting3 Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> orderes_queue.hpp fix Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com> Formatting4 Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

bader · 2020-05-21T12:49:33Z

sycl/test/warnings/warnings.cpp

@@ -1,4 +1,4 @@
-// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
+// RUN: %clangxx -fsycl --no-system-header-prefix=CL/ -Wall -Wextra -Wno-ignored-attributes -Wno-deprecated-declarations -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version %s -o %t.out


Minor note: --no-system-header-prefix=CL/ will most likely enable this check in non-SYCL headers e.g. OpenCL/L0.

I would also add -c option to save on linking time.

I will do it by separate PR.

If you want to save compile time, wouldn't "-fsyntax-only" be even better than "-c" ?

fadeeval · 2020-05-21T13:52:35Z

@turinevgeny, @v-klochkov, @MrSidims, @againull, @smaslov-intel, @AlexeySachkov, @Pennycook, @jbrodman, approve please if no objections

…ntel/llvm-test-suite#1596) intel#1583 introduced a clang-format issue. This fixes the issue. --------- Signed-off-by: Maronas, Marcos <marcos.maronas@intel.com>

fadeeval requested review from againull, AlexeySachkov, jbrodman, MrSidims, Pennycook, smaslov-intel, turinevgeny and a team as code owners April 24, 2020 13:17

fadeeval requested a review from s-kanaev April 24, 2020 13:17

Pennycook requested changes Apr 24, 2020

View reviewed changes

sycl/include/CL/sycl/intel/sub_group_host.hpp Outdated Show resolved Hide resolved

bader reviewed Apr 24, 2020

View reviewed changes

sycl/include/CL/sycl/program.hpp Outdated Show resolved Hide resolved

bader requested changes Apr 24, 2020

View reviewed changes

fadeeval force-pushed the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch from 04ab3cb to 45b2d41 Compare April 24, 2020 15:22

romanovvlad previously requested changes Apr 27, 2020

View reviewed changes

fadeeval changed the title ~~[SYCL] Handle massive warnings comming from dpcxx lib~~ [SYCL] Handle massive warnings comming from DPCPP lib Apr 27, 2020

s-kanaev reviewed Apr 28, 2020

View reviewed changes

fadeeval force-pushed the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch from f53df61 to 067e293 Compare April 28, 2020 15:22

bader reviewed Apr 29, 2020

View reviewed changes

sycl/include/CL/sycl/detail/defines.hpp Outdated Show resolved Hide resolved

bader mentioned this pull request Apr 30, 2020

[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT #1575

Merged

bader reviewed Apr 30, 2020

View reviewed changes

alexbatashev mentioned this pull request May 1, 2020

[SYCL] Fix getDeviceFromHandler declarations #1626

Merged

fadeeval force-pushed the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch from a2351d8 to c9fdf67 Compare May 12, 2020 11:32

fadeeval requested a review from v-klochkov as a code owner May 12, 2020 11:32

fadeeval force-pushed the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch 4 times, most recently from 71ad83b to 00f94fc Compare May 19, 2020 10:44

fadeeval dismissed stale reviews from MrSidims, Pennycook, and bader via c3f1836 May 20, 2020 09:35

fadeeval force-pushed the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch from 3a74b50 to c3f1836 Compare May 20, 2020 09:35

smaslov-intel previously approved these changes May 20, 2020

View reviewed changes

againull previously approved these changes May 20, 2020

View reviewed changes

fadeeval dismissed stale reviews from againull and smaslov-intel via de44543 May 21, 2020 07:26

fadeeval requested a review from a team as a code owner May 21, 2020 07:26

bader requested changes May 21, 2020

View reviewed changes

fadeeval added 8 commits May 21, 2020 15:32

warnings.cpp fix

191be27

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Warnings fix

0a6b77d

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

Warnings fix 2

ac59ae0

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

rebase fix

81d2657

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

warnings.cpp RUN fix

0ece5d9

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

lit.cfg.py fix

a3eb3d8

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

warnings.cpp fix 2

735e7b8

Signed-off-by: Aleksander Fadeev <aleksander.fadeev@intel.com>

fadeeval force-pushed the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch from 8b5b08a to 735e7b8 Compare May 21, 2020 12:42

bader approved these changes May 21, 2020

View reviewed changes

bader reviewed May 21, 2020

View reviewed changes

Pennycook approved these changes May 21, 2020

View reviewed changes

bader merged commit e0f59e3 into intel:sycl May 21, 2020

fadeeval mentioned this pull request May 26, 2020

[SYCL] Fix one more warning coming from SYCL headers #1758

Merged

fadeeval deleted the private/fadeeval/Massive_warnings_comming_from_DPCXX_lib branch June 1, 2020 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Handle massive warnings comming from DPCPP lib #1583

[SYCL] Handle massive warnings comming from DPCPP lib #1583

fadeeval commented Apr 24, 2020 •

edited

Loading

bader left a comment

bader Apr 24, 2020

romanovvlad left a comment

romanovvlad Apr 27, 2020

romanovvlad Apr 27, 2020

fadeeval Apr 28, 2020

s-kanaev left a comment

bader commented Apr 28, 2020

bader Apr 30, 2020

bader Apr 30, 2020

bader Apr 30, 2020

fadeeval Apr 30, 2020 •

edited

Loading

fadeeval Apr 30, 2020

bader May 21, 2020

bader May 21, 2020

fadeeval May 21, 2020

v-klochkov May 21, 2020

fadeeval commented May 21, 2020

		@@ -1,4 +1,4 @@
		// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
		// RUN: %clangxx -I /localdisk2/icl/fadeeval/sycl_workspace/build/include/sycl -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version %s -o %t.out

		@@ -263,7 +263,16 @@ class __SYCL_EXPORT_DEPRECATED("Replaced by in_order queue property")

		namespace std {
		template <> struct hash<cl::sycl::ordered_queue> {

		@@ -1,4 +1,4 @@
		// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
		// RUN: %clangxx -fsycl -I %sycl_source_libs_dir -Wall -Wextra -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version %s -o %t.out

		@@ -2,4 +2,3 @@

		; REQUIRES: shell
		; UNSUPPORTED: asan, ubsan, msan

		@@ -1,4 +1,4 @@
		// RUN: %clangxx -Wall -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version -fsycl %s -o %t.out
		// RUN: %clangxx -fsycl --no-system-header-prefix=CL/ -Wall -Wextra -Wno-ignored-attributes -Wno-deprecated-declarations -Wpessimizing-move -Wunused-variable -Wmismatched-tags -Wunneeded-internal-declaration -Werror -Wno-unknown-cuda-version %s -o %t.out

[SYCL] Handle massive warnings comming from DPCPP lib #1583

[SYCL] Handle massive warnings comming from DPCPP lib #1583

Conversation

fadeeval commented Apr 24, 2020 • edited Loading

bader left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanovvlad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s-kanaev left a comment

Choose a reason for hiding this comment

bader commented Apr 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fadeeval Apr 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fadeeval commented May 21, 2020

fadeeval commented Apr 24, 2020 •

edited

Loading

fadeeval Apr 30, 2020 •

edited

Loading