Thanks Zhigang, I looked a little deeper into make -j4 issue and found the reason is that update_blob_ocl_header.py is designed for single process. To simplify cmake script, agree to adjust it.
Glad to know the command to check trailing whitespace, something was missed by my manually check, :( Thanks Yejun -----Original Message----- From: Zhigang Gong [mailto:[email protected]] Sent: Thursday, February 13, 2014 3:11 PM To: Guo, Yejun Cc: [email protected] Subject: Re: [Beignet] [PATCH] BGE: add param to switch the behavior of math func One typo in the subject, should use GBE rather than BGE. Some comments as below: On Thu, Feb 13, 2014 at 06:14:37AM +0800, Guo Yejun wrote: > Add OCL_STRICT_CONFORMANCE to switch the behavior of math func, > The funcs will be high precision with perf drops if it is 1, > Fast path with good enough precision will be selected if it is 0. > > This change is to add the code basis, with 'sin' implmented as > an example, other math functions support will be added later. > > Signed-off-by: Guo Yejun <[email protected]> > --- > backend/CMakeLists.txt | 3 ++- > backend/src/CMakeLists.txt | 18 +++++++++++++++++- > backend/src/GBEConfig.h.in | 1 + > backend/src/backend/program.cpp | 12 +++++++++++- > backend/src/ocl_stdlib.tmpl.h | 27 +++++++++++++++++++++++++++ > utests/setenv.sh.in | 1 + > 6 files changed, 59 insertions(+), 3 deletions(-) > > diff --git a/backend/CMakeLists.txt b/backend/CMakeLists.txt > index dd55a4a..08122da 100644 > --- a/backend/CMakeLists.txt > +++ b/backend/CMakeLists.txt > @@ -98,8 +98,9 @@ include_directories (${CMAKE_CURRENT_BINARY_DIR}) > ############################################################## > add_subdirectory (src) > set(LOCAL_PCH_OBJECT_DIR ${LOCAL_PCH_OBJECT_DIR} PARENT_SCOPE) > +set(LOCAL_PCH_STRICT_OBJECT_DIR ${LOCAL_PCH_STRICT_OBJECT_DIR} PARENT_SCOPE) > set(LOCAL_PCM_OBJECT_DIR ${LOCAL_PCM_OBJECT_DIR} PARENT_SCOPE) > set (GBE_BIN_GENERATER > - OCL_PCM_PATH=${LOCAL_PCM_OBJECT_DIR} > OCL_PCH_PATH=${LOCAL_PCH_OBJECT_DIR} > ${CMAKE_CURRENT_BINARY_DIR}/src/gbe_bin_generater > + OCL_PCM_PATH=${LOCAL_PCM_OBJECT_DIR} > OCL_PCH_PATH=${LOCAL_PCH_OBJECT_DIR} > OCL_PCH_STRICT_PATH=${LOCAL_PCH_STRICT_OBJECT_DIR} > ${CMAKE_CURRENT_BINARY_DIR}/src/gbe_bin_generater > PARENT_SCOPE) > > diff --git a/backend/src/CMakeLists.txt b/backend/src/CMakeLists.txt > index 33494a0..775f19b 100644 > --- a/backend/src/CMakeLists.txt > +++ b/backend/src/CMakeLists.txt > @@ -43,6 +43,8 @@ add_custom_command( > > set (pch_object ${ocl_blob_file}.pch) > set (local_pch_object ${ocl_blob_file}.local.pch) > +set (pch_strict_object ${ocl_blob_file}.strict.pch) > +set (local_pch_strict_object ${ocl_blob_file}.strict.local.pch) > # generate pch object > if (LLVM_VERSION_NODOT VERSION_GREATER 32) > set (clang_cmd -cc1 -x cl -triple spir -ffp-contract=off) > @@ -66,6 +68,17 @@ add_custom_command( > add_custom_target(pch_object > DEPENDS ${pch_object}) > > +add_custom_command( > + OUTPUT ${pch_strict_object} > + COMMAND rm -f ${pch_strict_object} > + COMMAND clang ${clang_cmd} -DOCL_STRICT_CONFORMANCE=1 --relocatable-pch > -emit-pch -isysroot ${CMAKE_CURRENT_BINARY_DIR} ${ocl_blob_file} -o > ${pch_strict_object} > + COMMAND clang ${clang_cmd} -DOCL_STRICT_CONFORMANCE=1 -emit-pch > ${ocl_blob_file} -o ${local_pch_strict_object} > + DEPENDS ${ocl_blob_file} > + ) > + > +add_custom_target(pch_strict_object > + DEPENDS ${pch_strict_object}) > + Should not set pch_strict_object as another target, you just need to add the commands using to generate pch_strict_object and local_pch_strict_object to the place after the normal pch_object/local_pch_object generation. As both objects depend on the same file set, this can simplfy the cmake script and could avoid the -j4 issue. You can try to use make -j4 with you current patch. It will fail. > macro(ll_add_library ll_lib ll_sources) > foreach (ll ${${ll_sources}}) > add_custom_command( > @@ -176,7 +189,7 @@ set (pcm_lib "beignet.bc") > set (pcm_sources ocl_barrier.ll ocl_memset.ll ocl_memcpy.ll) > ll_add_library (${pcm_lib} pcm_sources) > > -ADD_DEPENDENCIES (gbe pch_object ${pcm_lib}) > +ADD_DEPENDENCIES (gbe pch_object pch_strict_object ${pcm_lib}) Just delete the pch_strict_object here. > target_link_libraries( > gbe > ${DRM_INTEL_LIBRARY} > @@ -196,13 +209,16 @@ TARGET_LINK_LIBRARIES(gbe_bin_generater gbe) > #install (FILES backend/program.h DESTINATION include/gen) > install (FILES ${ocl_blob_file} DESTINATION ${LIB_INSTALL_DIR}/beignet) > install (FILES ${pch_object} DESTINATION ${LIB_INSTALL_DIR}/beignet) > +install (FILES ${pch_strict_object} DESTINATION ${LIB_INSTALL_DIR}/beignet) > install (FILES ${CMAKE_CURRENT_BINARY_DIR}/${pcm_lib} DESTINATION > ${LIB_INSTALL_DIR}/beignet) > # When build beignet itself, we need to export the local precompiled header > file and precompiled module > # file to libcl and utests. > set (LOCAL_PCH_OBJECT_DIR > "${local_pch_object}:${beignet_install_path}/ocl_stdlib.h.pch" PARENT_SCOPE) > +set (LOCAL_PCH_STRICT_OBJECT_DIR > "${local_pch_object}:${beignet_install_path}/ocl_stdlib.h.strict.pch" > PARENT_SCOPE) ~~~~~~~~~~~~~~~~ should use local_pch_strict_object. And you may need to use git show --check and fix all the format issue before submit the patch to the list. Take this patch as an example, git show --check found the following warnings: backend/src/ocl_stdlib.tmpl.h:4467: trailing whitespace. +/// It is required by OpenCL that built-in math functions (without native_/half_) backend/src/ocl_stdlib.tmpl.h:4468: trailing whitespace. +/// have high precision, but GPU hardware is designed to be good enough precision, backend/src/ocl_stdlib.tmpl.h:4474: trailing whitespace. +/// of the math functions. _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
