[SYCL] Handle massive warnings comming from DPCPP lib by fadeeval · Pull Request #1583 · intel/llvm

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

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

Please, clang-format the patch.

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

sycl/test/warnings/warnings.cpp

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

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

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

sycl/test/CMakeLists.txt

There is already sycl_inc_dir varible for that.

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

sycl/test/warnings/warnings.cpp

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

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 a team, AlexeySachkov, MrSidims, Pennycook, againull, jbrodman, smaslov-intel and turinevgeny 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

sycl/include/CL/sycl/ordered_queue.hpp Outdated

Copy link

Contributor

bader Apr 30, 2020

Choose a reason for hiding this comment

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

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

bader reviewed Apr 30, 2020

View reviewed changes

sycl/test/CMakeLists.txt Outdated

Copy link

Contributor

bader Apr 30, 2020

Choose a reason for hiding this comment

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

There is already sycl_inc_dir varible for that.

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

llvm/test/Bindings/Go/go.test Outdated

Copy link

Contributor

bader May 21, 2020

Choose a reason for hiding this comment

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

Please, remove all unrelated changes from your patch.

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

	// 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

		@@ -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

Conversation

fadeeval commented Apr 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romanovvlad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s-kanaev left a comment

Choose a reason for hiding this comment

Uh oh!

bader commented Apr 28, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fadeeval Apr 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fadeeval commented May 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

fadeeval commented Apr 24, 2020 •

edited

Loading

fadeeval Apr 30, 2020 •

edited

Loading