[SYCL][CUDA] Add missing dependency on OpenCL headers #1260

fwyzard · 2020-03-06T08:59:44Z

This should fix the build process, where make sycl-toolchain -j would sometimes fail due to pi_cuda and ocl-headers building in parallel.

Signed-off-by: Andrea Bocci <[email protected]>

fwyzard · 2020-03-06T09:01:46Z

Hopefully fixes #1211.

bader · 2020-03-06T09:14:32Z

Tagging @Ruyk, @bjoernknafla, @steffenlarsen.
Why would we need opencl-headers for CUDA plug-in at all?

fwyzard · 2020-03-06T09:59:41Z

Here's the dependency chain I see:

pi_cuda.hpp
⇢ #include <CL/sycl/detail/pi.h>

pi_cuda.cpp
⇢ #include <CL/sycl/detail/pi.hpp>

CL/sycl/detail/pi.hpp
⇢ #include <CL/sycl/detail/common.hpp>
⇢ #include <CL/sycl/detail/pi.h>

CL/sycl/detail/pi.h
⇢ #include <CL/cl_usm_ext.h>
⇢ #include <CL/opencl.h>

CL/cl_usm_ext.h
⇢ #include <CL/cl.h>

CL/sycl/detail/common.hpp
⇢ #include <CL/cl.h>
⇢ #include <CL/cl_ext.h>
⇢ #include <CL/cl_ext_intel.h>

So, the CUDA plugin depends on the pi.h/pi.hpp headers, which in turn depend on the OpenCL headers.

bader · 2020-03-06T10:05:53Z

This looks like a bug to me. CUDA plugin should include PI declarations, which are independent from OpenCL.

bader

I'm okay with the fix, but we need to think about removing the PI dependency on OpenCL headers.

steffenlarsen · 2020-03-06T10:36:11Z

This looks like a bug to me. CUDA plugin should include PI declarations, which are independent from OpenCL.

The PI declarations are mostly independent from OpenCL, except for a mapping from the PI descriptors to the corresponding OpenCL ones, e.g. mapping between cl_device_type and _pi_device_type defined as

typedef enum : pi_uint64 {
  PI_DEVICE_TYPE_CPU = CL_DEVICE_TYPE_CPU,
  PI_DEVICE_TYPE_GPU = CL_DEVICE_TYPE_GPU,
  PI_DEVICE_TYPE_ACC = CL_DEVICE_TYPE_ACCELERATOR
} _pi_device_type;

Ruyk · 2020-03-06T12:43:37Z

There are also various places in the SYCL RT that simply assume CL types, such as the interop functions, so the opencl-headers are required to build the project anyway.
Without pulling the opencl-headers, the build system in some cases picks up the OpenCL 1.2 headers from NVIDIA and creates more build problems.
Is it a major problem if the dependency is there @bader ?

Ruyk · 2020-03-06T12:46:14Z

Hopefully fixes #1211.

I tried that fix locally but didn't change anything in the dependency graph on cmake. The problem in #1211 never happens for me locally so I cannot check, but let us know if it does.

bader · 2020-03-06T14:56:44Z

There are also various places in the SYCL RT that simply assume CL types, such as the interop functions, so the opencl-headers are required to build the project anyway.

Things like this are plug-in specific and should reside in plug-in specific headers:
pi-opencl.h <-- interop with OpenCL
pi-cuda.h <-- interop with CUDA

Without pulling the opencl-headers, the build system in some cases picks up the OpenCL 1.2 headers from NVIDIA and creates more build problems.

We should use only CUDA plug-in on platform and this plug-in should not include OpenCL headers. This will resolve the problem.

Is it a major problem if the dependency is there @bader ?

No for this patch. I already approved it.

sycl/plugins/cuda/CMakeLists.txt

fwyzard · 2020-03-06T15:07:13Z

I don't see any change in the graph, either.

However without this change, running make -j16 sycl-toolchain opencl-aot results in

Scanning dependencies of target ocl-headers
Scanning dependencies of target llvm_vcsrevision_h
Scanning dependencies of target LLVMDemangle
Scanning dependencies of target LLVMTableGenGlobalISel
Scanning dependencies of target sycl-headers
Scanning dependencies of target obj.llvm-tblgen
Scanning dependencies of target obj.clang-tblgen
Scanning dependencies of target generate_convert_clc.cl
Scanning dependencies of target pi_cuda
[  0%] Creating directories for 'ocl-headers'
[  0%] Generating VCSRevision.h
[  0%] Copying SYCL headers ...
[  0%] Generating convert-clc.cl
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/MicrosoftDemangleNodes.cpp.o
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/MicrosoftDemangle.cpp.o
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/Demangle.cpp.o
[  0%] Building CXX object tools/sycl/plugins/cuda/CMakeFiles/pi_cuda.dir/pi_cuda.cpp.o
-- Found Git: /usr/bin/git (found version "2.18.2") 
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/CodeExpander.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDag.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagInstr.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagEdge.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagOperands.cpp.o
[  0%] Built target sycl-headers
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangASTNodesEmitter.cpp.o
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ASTTableGen.cpp.o
[  0%] Built target generate_convert_clc.cl
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmWriterEmitter.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmMatcherEmitter.cpp.o
[  0%] Built target llvm_vcsrevision_h
[  0%] Performing download step (git clone) for 'ocl-headers'
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmWriterInst.cpp.o
Cloning into 'inc'...
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/Attributes.cpp.o
In file included from /data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/exception.hpp:14,
                 from /data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/common.hpp:64,
                 from /data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.hpp:13,
                 from /data/user/fwyzard/sycl/llvm/sycl/plugins/cuda/pi_cuda.cpp:10:
/data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.h:209:7: error: 'CL_DEVICE_QUEUE_ON_DEVICE_PROPERTIES' was not declared in this scope
       CL_DEVICE_QUEUE_ON_DEVICE_PROPERTIES,
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.h:209:7: note: suggested alternative: 'CL_DEVICE_QUEUE_PROPERTIES'
       CL_DEVICE_QUEUE_ON_DEVICE_PROPERTIES,
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       CL_DEVICE_QUEUE_PROPERTIES
/data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.h:210:45: error: 'CL_DEVICE_QUEUE_ON_HOST_PROPERTIES' was not declared in this scope
   PI_DEVICE_INFO_QUEUE_ON_HOST_PROPERTIES = CL_DEVICE_QUEUE_ON_HOST_PROPERTIES,
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.h:210:45: note: suggested alternative: 'CL_DEVICE_QUEUE_PROPERTIES'
   PI_DEVICE_INFO_QUEUE_ON_HOST_PROPERTIES = CL_DEVICE_QUEUE_ON_HOST_PROPERTIES,
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                             CL_DEVICE_QUEUE_PROPERTIES
/data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.h:302:33: error: 'CL_ABGR' was not declared in this scope
   PI_IMAGE_CHANNEL_ORDER_ABGR = CL_ABGR,
                                 ^~~~~~~
/data/user/fwyzard/sycl/llvm/sycl/include/CL/sycl/detail/pi.h:302:33: note: suggested alternative: 'CL_ARGB'
   PI_IMAGE_CHANNEL_ORDER_ABGR = CL_ABGR,
                                 ^~~~~~~
                                 CL_ARGB
...

and so on - which I assume comes from picking up the CUDA OpenCL headers.

With this change, the same command results in

Scanning dependencies of target llvm_vcsrevision_h
Scanning dependencies of target LLVMDemangle
Scanning dependencies of target ocl-headers
Scanning dependencies of target LLVMTableGenGlobalISel
Scanning dependencies of target generate_convert_clc.cl
Scanning dependencies of target sycl-headers
Scanning dependencies of target obj.llvm-tblgen
Scanning dependencies of target obj.clang-tblgen
[  0%] Creating directories for 'ocl-headers'
[  0%] Generating VCSRevision.h
[  0%] Generating convert-clc.cl
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/Demangle.cpp.o
[  0%] Copying SYCL headers ...
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/MicrosoftDemangleNodes.cpp.o
[  0%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/MicrosoftDemangle.cpp.o
-- Found Git: /usr/bin/git (found version "2.18.2") 
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagEdge.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/CodeExpander.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDag.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagOperands.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagPredicate.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagInstr.cpp.o
[  0%] Built target sycl-headers
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ASTTableGen.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchDagPredicateDependencyEdge.cpp.o
[  0%] Built target generate_convert_clc.cl
[  0%] Built target llvm_vcsrevision_h
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangASTNodesEmitter.cpp.o
[  0%] Building CXX object utils/TableGen/GlobalISel/CMakeFiles/LLVMTableGenGlobalISel.dir/GIMatchTree.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmMatcherEmitter.cpp.o
[  0%] Performing download step (git clone) for 'ocl-headers'
Cloning into 'inc'...
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmWriterEmitter.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmWriterInst.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/Attributes.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CallingConvEmitter.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeEmitterGen.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenDAGPatterns.cpp.o
Note: checking out 'origin/master'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at 96f5bde add experimental enums (#70)
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenHwModes.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenInstruction.cpp.o
[  0%] No patch step for 'ocl-headers'
[  0%] Performing update step for 'ocl-headers'
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangASTPropertiesEmitter.cpp.o
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangAttrEmitter.cpp.o
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangCommentCommandInfoEmitter.cpp.o
HEAD is up to date.
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenMapTable.cpp.o
[  0%] No configure step for 'ocl-headers'
[  0%] Performing build step for 'ocl-headers'
[  0%] No install step for 'ocl-headers'
[  0%] Completed 'ocl-headers'
[  0%] Built target ocl-headers
Scanning dependencies of target ocl-icd
[  0%] Creating directories for 'ocl-icd'
[  0%] Performing download step (git clone) for 'ocl-icd'
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp.o
Cloning into 'icd'...
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangCommentHTMLTagsEmitter.cpp.o
[  0%] Linking CXX shared library ../../lib64/libLLVMDemangle.so
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenRegisters.cpp.o
[  0%] Built target LLVMDemangle
Scanning dependencies of target pi_cuda
[  0%] Building CXX object tools/sycl/plugins/cuda/CMakeFiles/pi_cuda.dir/pi_cuda.cpp.o
Note: checking out 'origin/master'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at c7fda8b Corrected inconsistent indentation
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenSchedule.cpp.o
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenTarget.cpp.o
[  0%] No patch step for 'ocl-icd'
[  0%] Performing update step for 'ocl-icd'
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/DAGISelEmitter.cpp.o
...
HEAD is up to date.
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangDataCollectorsEmitter.cpp.o
[  0%] Performing configure step for 'ocl-icd'
-- The C compiler identification is GNU 8.3.1
-- The CXX compiler identification is GNU 8.3.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
[  0%] Building CXX object tools/clang/utils/TableGen/CMakeFiles/obj.clang-tblgen.dir/ClangDiagnosticsEmitter.cpp.o
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
[  0%] Linking CXX shared library ../../../../lib64/libpi_cuda.so
[  0%] Built target pi_cuda
...

In the second case, pi_cuda is built only after the OpenCL headers have been set up, and the build succeeds.

Ruyk · 2020-03-06T15:21:45Z

Created #1265 to keep track of the opencl-header dependency issue.

Ruyk · 2020-03-06T15:24:03Z

I don't see any change in the graph, either.

That is my concern, that maybe is just delaying the point where the dependency is required and just so happen to work. But, if it is solving #1211 , then it should be fine.

bjoernknafla · 2020-03-06T15:25:28Z

sycl/plugins/cuda/CMakeLists.txt

@@ -21,6 +21,10 @@ add_library(pi_cuda SHARED
  "pi_cuda.cpp"
 )

+add_dependencies(pi_cuda
+  ocl-headers
+)


Another solution could be to make the llvm/sycl/CMakeLists.txt OpenCL-Headers library (used below) dependent on the custom target ocl-headers - though without testing I am not 100% sure if that would resolve this issue, too.

It might not.
Basically we need to make sure that ocl-headers target to "built" before we start compilation of CUDA plug-in.

To chime in, a bit late...

This was indeed the correct solution in my mind. Setting dependencies on interface libraries (which OpenCL-Headers is) is not possible. CMake just silently doesn't do anything for the dependency in that case. (I've been bit by this before...)

[SYCL][CUDA] Add missing dependency on OpenCL headers

5f9f469

Signed-off-by: Andrea Bocci <[email protected]>

fwyzard force-pushed the cuda_add_dependency_on_opencl_headers branch from fa348dc to 5f9f469 Compare March 6, 2020 09:01

fwyzard changed the title ~~[CUDA] add missing dependency on OpenCL headers~~ [SYCL][CUDA] add missing dependency on OpenCL headers Mar 6, 2020

fwyzard changed the title ~~[SYCL][CUDA] add missing dependency on OpenCL headers~~ [SYCL][CUDA] Add missing dependency on OpenCL headers Mar 6, 2020

bader linked an issue Mar 6, 2020 that may be closed by this pull request

Make fails the first time it is called after cmake #1211

Closed

bader added the cuda CUDA back-end label Mar 6, 2020

bader approved these changes Mar 6, 2020

View reviewed changes

romanovvlad requested a review from smaslov-intel March 6, 2020 12:50

bader reviewed Mar 6, 2020

View reviewed changes

sycl/plugins/cuda/CMakeLists.txt Show resolved Hide resolved

Ruyk mentioned this pull request Mar 6, 2020

[SYCL][CUDA] Remove opencl-headers dependency from pi-cuda.h #1265

Closed

bjoernknafla reviewed Mar 6, 2020

View reviewed changes

bader merged commit bed858b into intel:sycl Mar 6, 2020

fwyzard deleted the cuda_add_dependency_on_opencl_headers branch March 7, 2020 00:49

krasznaa mentioned this pull request Mar 11, 2020

libOpenCL not being installed with LLVM/clang #1285

Closed

[SYCL][CUDA] Add missing dependency on OpenCL headers #1260

[SYCL][CUDA] Add missing dependency on OpenCL headers #1260

Uh oh!

Conversation

fwyzard commented Mar 6, 2020

Uh oh!

fwyzard commented Mar 6, 2020

Uh oh!

bader commented Mar 6, 2020

Uh oh!

fwyzard commented Mar 6, 2020

Uh oh!

bader commented Mar 6, 2020

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

steffenlarsen commented Mar 6, 2020

Uh oh!

Ruyk commented Mar 6, 2020

Uh oh!

Ruyk commented Mar 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bader commented Mar 6, 2020

Uh oh!

Uh oh!

fwyzard commented Mar 6, 2020

Uh oh!

Ruyk commented Mar 6, 2020

Uh oh!

Ruyk commented Mar 6, 2020

Uh oh!

bjoernknafla Mar 6, 2020

Choose a reason for hiding this comment

Uh oh!

bader Mar 6, 2020

Choose a reason for hiding this comment

Uh oh!

krasznaa Mar 6, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Ruyk commented Mar 6, 2020 •

edited

Loading