[Beignet] [PATCH V2] Add the PCH support when building the source.

Zhigang Gong zhigang.gong at linux.intel.com
Thu Jul 18 04:25:25 PDT 2013


Junyan,

This patch is broken for me. Some comments as below:

On Wed, Jul 17, 2013 at 04:38:41PM +0800, junyan.he at inbox.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
> Because the utest grows bigger, the runtime compiling time seems a big
> problem to cause the utest_run very slow.  Most of the time wastes on
> re-parsing the big header file such as ocl_stdlib.h.
> Using the PCH feature of Clang can shorten the compiling time a lot.
> Because we build the pch file at building time, but some source
> has runtime building option for LLVM, and these options may cause
> the compitable problem between the pch header and the CL source code.
> We fallback to build the whole header and source when we find the
> options are not compitable.
> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  backend/src/CMakeLists.txt      |   31 +++++++++++++++++++++++++++++--
>  backend/src/backend/program.cpp |   36 ++++++++++++++++++++++++++++++++----
>  2 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/backend/src/CMakeLists.txt b/backend/src/CMakeLists.txt
> index a0fe198..04b4202 100644
> --- a/backend/src/CMakeLists.txt
> +++ b/backend/src/CMakeLists.txt
> @@ -1,5 +1,4 @@
>  #add_subdirectory(llvm)
> -
>  macro (stringify TO_STRINGIFY_PATH TO_STRINGIFY_FILES)
>  foreach (to_stringify_file ${TO_STRINGIFY_FILES})
>    set (input_file ${TO_STRINGIFY_PATH}/${to_stringify_file}.h)
> @@ -20,9 +19,36 @@ foreach (to_stringify_file ${TO_STRINGIFY_FILES})
>  endforeach (to_stringify_file)
>  endmacro (stringify)
>  
> +macro (generate_pch TO_GENERATE_PATH SRC_HEADERS OUTPUT_HEADER)
> +    if (LLVM_VERSION_NODOT VERSION_GREATER 32)
> +	set (clang_cmd -cc1 -x cl -triple spir -ffp-contract=off -emit-pch)
> +    else (LLVM_VERSION_NODOT VERSION_GREATER 32)
> +	if (LLVM_VERSION_NODOT VERSION_GREATER 31)
> +	    set (clang_cmd -cc1 -x cl -triple nvptx -ffp-contract=off -emit-pch)
> +	else (LLVM_VERSION_NODOT VERSION_GREATER 31)
> +	    set (clang_cmd -cc1 -x cl -triple ptx32 -emit-pch)
> +	endif (LLVM_VERSION_NODOT VERSION_GREATER 31)
> +    endif (LLVM_VERSION_NODOT VERSION_GREATER 32)
> +    set (output_file ${TO_GENERATE_PATH}/${OUTPUT_HEADER})
> +    set (output_pch ${TO_GENERATE_PATH}/${OUTPUT_HEADER}.pch)
> +    add_custom_command(
> +	OUTPUT ${output_file}
> +	COMMAND rm -rf ${output_file}
> +	COMMAND rm -rf ${output_pch}
> +	COMMAND cat ${SRC_HEADERS} > ${output_file}
The above SRC_HEADERS are a set of file names without path, I doubt it could work as
you want. Need to do two things to make it work, one is to add the path information,
the second is to cat each file one by one. 
> +	COMMAND clang ${clang_cmd} ${output_file} -o ${output_pch}
You may need to add some code here to check whether the above command excute successfully.
> +	MAIN_DEPENDENCY ${SRC_HEADERS}
The similar issue is also in above SRC_HEADERS. It should have path information.
Otherwise, it may not generate correct dependency rules.
> +	)
> +endmacro (generate_pch)
> +
> +
>  set (TO_STRINGIFY_FILES ocl_stdlib ocl_common_defines)
>  stringify ("${GBE_SOURCE_DIR}/src/" "${TO_STRINGIFY_FILES}")
>  
> +set (PCH_DEP_HEADERS ocl_common_defines.h ocl_stdlib.h)
> +set (PCH_HEADER ocl_header.h)
> +generate_pch ("${GBE_SOURCE_DIR}/src" "${PCH_DEP_HEADERS}" ${PCH_HEADER})
So you want to generate the pch file to the GBE_SOURCE_DIR/src directory.
Right? Then latter, if you didn't excute make install, the pch file should
be still in that directory. And I haven't found any code in this patch to
handle that situation.

> +
>  if (GBE_USE_BLOB)
>    set (GBE_SRC
>         blob.cpp
> @@ -31,7 +57,7 @@ else (GBE_USE_BLOB)
>    set (GBE_SRC
>      ocl_stdlib.h
>      ocl_stdlib_str.cpp  # this file is auto-generated.
> -    ocl_stdlib_str.cpp
> +    ocl_header.h
After this patch, we no longer need to add ocl_stdlib_str.cpp
or any other generated files to the GBE_SRC, as we will carry those
information in a separated pch file, right? It should be safe to
delete those ocl_xxx.[cpp|h] here. Right?

>      ocl_common_defines.h
>      ocl_common_defines_str.cpp # this file is auto-generated.
>      sys/vector.hpp
> @@ -122,5 +148,6 @@ target_link_libraries(
>                        ${CMAKE_DL_LIBS})
>  
>  install (TARGETS gbe LIBRARY DESTINATION lib)
> +install (FILES ocl_header.h.pch DESTINATION lib)
>  install (FILES backend/program.h DESTINATION include/gen)
>  
> diff --git a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp
> index 2a4feb9..7f9b36b 100644
> --- a/backend/src/backend/program.cpp
> +++ b/backend/src/backend/program.cpp
> @@ -35,6 +35,7 @@
>  #include <cstring>
>  #include <algorithm>
>  #include <fstream>
> +#include <dlfcn.h>
>  
>  /* Not defined for LLVM 3.0 */
>  #if !defined(LLVM_VERSION_MAJOR)
> @@ -222,25 +223,52 @@ namespace gbe {
>  
>    extern std::string ocl_stdlib_str;
>    extern std::string ocl_common_defines_str;
> +
>    static gbe_program programNewFromSource(const char *source,
>                                            size_t stringSize,
>                                            const char *options,
>                                            char *err,
>                                            size_t *errSize)
>    {
> +    Dl_info info;
>      char clStr[L_tmpnam+1], llStr[L_tmpnam+1];
>      const std::string clName = std::string(tmpnam_r(clStr)) + ".cl"; /* unsafe! */
>      const std::string llName = std::string(tmpnam_r(llStr)) + ".ll"; /* unsafe! */
> +    std::string pchHeaderName;
> +    std::string clOpt;
>  
> -    // Write the source to the cl file
>      FILE *clFile = fopen(clName.c_str(), "w");
>      FATAL_IF(clFile == NULL, "Failed to open temporary file");
> -    fwrite(ocl_common_defines_str.c_str(), strlen(ocl_common_defines_str.c_str()), 1, clFile);
> -    fwrite(ocl_stdlib_str.c_str(), strlen(ocl_stdlib_str.c_str()), 1, clFile);
> +
> +    if(options)
> +      clOpt += options;
> +
> +    if (::strstr(options, "-cl-std")) {
The options may be null, you need to change it to if (options && ...)

> +      /* Some building option may cause the prebuild pch header file
> +         not compatible with the XXX.cl source. We need rebuild all here.*/
> +      fwrite(ocl_common_defines_str.c_str(), strlen(ocl_common_defines_str.c_str()), 1, clFile);
> +      fwrite(ocl_stdlib_str.c_str(), strlen(ocl_stdlib_str.c_str()), 1, clFile);         
> +    } else {
> +      /* Get the path of the shared lib of ourself, the pch file
> +         always exists in the same folder. */
> +      ::dladdr((const void*)programNewFromSource, &info);
> +      llvm::StringRef str(info.dli_fname);
> +      FATAL_IF(!str.endswith("libgbe.so"), "Failed to find the path of our lib self");
Just as the above comment, you may need to copy the pch file to the corresponding .lib
directory at the build stage, thus it can work well when we haven't installed the libgbe.
Otherwise, I'm afriad it will fail to find the pch file.

> +      str = str.substr(0, str.rfind("/") + 1);
> +
> +      pchHeaderName = str.str();
> +      pchHeaderName += "ocl_header.h.pch";
> +      
> +      clOpt += "-include-pch ";
> +      clOpt += pchHeaderName;
> +      clOpt += " ";
> +    }
> +
> +    // Write the source to the cl file
>      fwrite(source, strlen(source), 1, clFile);
>      fclose(clFile);
>  
> -    buildModuleFromSource(clName.c_str(), llName.c_str(), options ? options : "");
> +    buildModuleFromSource(clName.c_str(), llName.c_str(), clOpt.c_str());
>      remove(clName.c_str());
>  
>      // Now build the program from llvm
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list