[Beignet] [PATCH] GBE: Use addRemappedFile to avoid creating temporary cl source file.
Yang, Rong R
rong.r.yang at intel.com
Tue Sep 8 01:10:56 PDT 2015
Is this remapped file virtual file? If it is not a virtual file, I am afraid it is not thread/process safe.
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Gong, Zhigang
> Sent: Tuesday, September 8, 2015 14:33
> To: Luo, Xionghu; beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] GBE: Use addRemappedFile to avoid creating
> temporary cl source file.
>
> > -----Original Message-----
> > From: Luo, Xionghu
> > Sent: Tuesday, September 8, 2015 2:09 PM
> > To: Gong, Zhigang; beignet at lists.freedesktop.org
> > Cc: Gong, Zhigang
> > Subject: RE: [Beignet] [PATCH] GBE: Use addRemappedFile to avoid
> > creating temporary cl source file.
> >
> > This patch LGTM except some questions.
> >
> > How didn't decide the name "stringInput.cl"?
> Not sure what's your meaning here?
>
> > And since this method works, we could also remap all the input headers
> > in API clCompileProgram to avoid create temp files under /tmp, anyway,
> > this could be processed in another patch.
> Right, the header files's processing in clCompileProgram could be refined by
> using the same method.
>
>
> >
> > Luo Xionghu
> > Best Regards
> >
> > -----Original Message-----
> > From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf
> > Of Zhigang Gong
> > Sent: Monday, August 31, 2015 2:30 PM
> > To: beignet at lists.freedesktop.org
> > Cc: Gong, Zhigang
> > Subject: [Beignet] [PATCH] GBE: Use addRemappedFile to avoid creating
> > temporary cl source file.
> >
> > LLVM provides powerful string-remapped feature which could be used to
> > map a string to an input file name, thus we don't need to create a
> > temporary cl source file any more.
> >
> > This patch not only make things much clear and avoid the unecessary
> > file creation. It only fixes some weird directory related problems.
> > Because beignet creates the temoprary file at the /tmp directory.
> > Then the clang will search the include files in that directory by
> > default, but the developer expects it to search the working directory
> > firstly. This causing two weird things:
> > 1. If a .cl file is including a .h file in the current directory, beignet
> > will not find it.
> >
> > 2. Even if the probram add a "-I." option manually, beignet will search /tmp
> > firstly, and if there is a .h file in /tmp/ with the eaxct same file
> > name, beignet will the file located in /tmp.
> >
> > Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> > ---
> > backend/src/backend/program.cpp | 40
> > ++++++++++------------------------------
> > 1 file changed, 10 insertions(+), 30 deletions(-)
> >
> > diff --git a/backend/src/backend/program.cpp
> > b/backend/src/backend/program.cpp index d9e6416..330bead 100644
> > --- a/backend/src/backend/program.cpp
> > +++ b/backend/src/backend/program.cpp
> > @@ -518,7 +518,7 @@ namespace gbe {
> > #ifdef GBE_COMPILER_AVAILABLE
> > BVAR(OCL_OUTPUT_BUILD_LOG, false);
> >
> > - static bool buildModuleFromSource(const char* input, llvm::Module**
> > out_module, llvm::LLVMContext* llvm_ctx,
> > + static bool buildModuleFromSource(const char *source,
> > + llvm::Module** out_module, llvm::LLVMContext* llvm_ctx,
> > std::string dumpLLVMFileName,
> > std::vector<std::string>& options, size_t stringSize, char *err,
> > size_t *errSize) {
> > // Arguments to pass to the clang frontend @@ -551,8 +551,7 @@
> > namespace gbe {
> > args.push_back("-triple");
> > args.push_back("spir");
> > #endif /* LLVM_VERSION_MINOR <= 2 */
> > - args.push_back(input);
> > -
> > + args.push_back("stringInput.cl");
> > args.push_back("-ffp-contract=off");
> >
> > // The compiler invocation needs a DiagnosticsEngine so it can
> > report problems @@ -574,6 +573,9 @@ namespace gbe {
> > &args[0],
> > &args[0] + args.size(),
> > Diags);
> > + llvm::StringRef srcString(source);
> > + (*CI).getPreprocessorOpts().addRemappedFile("stringInput.cl",
> > +
> > + llvm::MemoryBuffer::getMemBuffer(srcString).release());
> >
> > // Create the compiler instance
> > clang::CompilerInstance Clang;
> > @@ -670,7 +672,6 @@ namespace gbe {
> > std::vector<std::string>& clOpt,
> > std::string& dumpLLVMFileName,
> > std::string& dumpASMFileName,
> > - std::string& clName,
> > int& optLevel,
> > size_t stringSize,
> > char *err, @@ -781,21 +782,6 @@
> > namespace gbe {
> > }
> > }
> >
> > - char clStr[] = "/tmp/XXXXXX.cl";
> > - int clFd = mkstemps(clStr, 3);
> > - clName = std::string(clStr);
> > -
> > - FILE *clFile = fdopen(clFd, "w");
> > - FATAL_IF(clFile == NULL, "Failed to open temporary file");
> > - // XXX enable cl_khr_fp64 may cause some potential bugs.
> > - // we may need to revisit here latter when we want to support fp64
> > completely.
> > - // For now, as we don't support fp64 actually, just disable it by default.
> > -#if 0
> > - #define ENABLE_CL_KHR_FP64_STR "#pragma OPENCL EXTENSION
> > cl_khr_fp64 : enable\n"
> > - if (options && !strstr(const_cast<char *>(options), "-cl-std=CL1.1"))
> > - fwrite(ENABLE_CL_KHR_FP64_STR,
> > strlen(ENABLE_CL_KHR_FP64_STR), 1, clFile); -#endif
> > -
> > if (!findPCH || invalidPCH) {
> > clOpt.push_back("-include");
> > clOpt.push_back("ocl.h");
> > @@ -805,9 +791,6 @@ namespace gbe {
> > clOpt.push_back(pchFileName);
> > }
> >
> > - // Write the source to the cl file
> > - fwrite(source, strlen(source), 1, clFile);
> > - fclose(clFile);
> > return true;
> > }
> >
> > @@ -820,11 +803,10 @@ namespace gbe {
> > {
> > int optLevel = 1;
> > std::vector<std::string> clOpt;
> > - std::string clName;
> > std::string dumpLLVMFileName, dumpASMFileName;
> > if (!processSourceAndOption(source, options, NULL, clOpt,
> > dumpLLVMFileName, dumpASMFileName,
> > - clName, optLevel,
> > + optLevel,
> > stringSize, err, errSize))
> > return NULL;
> >
> > @@ -836,7 +818,7 @@ namespace gbe {
> > if (!llvm::llvm_is_multithreaded())
> > llvm_mutex.lock();
> >
> > - if (buildModuleFromSource(clName.c_str(), &out_module, llvm_ctx,
> > dumpLLVMFileName, clOpt,
> > + if (buildModuleFromSource(source, &out_module, llvm_ctx,
> > + dumpLLVMFileName, clOpt,
> > stringSize, err, errSize)) {
> > // Now build the program from llvm
> > size_t clangErrSize = 0;
> > @@ -862,7 +844,6 @@ namespace gbe {
> > if (!llvm::llvm_is_multithreaded())
> > llvm_mutex.unlock();
> >
> > - remove(clName.c_str());
> > return p;
> > }
> > #endif
> > @@ -879,10 +860,9 @@ namespace gbe {
> > {
> > int optLevel = 1;
> > std::vector<std::string> clOpt;
> > - std::string clName;
> > std::string dumpLLVMFileName, dumpASMFileName;
> > if (!processSourceAndOption(source, options, temp_header_path, clOpt,
> > - dumpLLVMFileName,
> > dumpASMFileName, clName,
> > + dumpLLVMFileName,
> > dumpASMFileName,
> > optLevel, stringSize, err, errSize))
> > return NULL;
> >
> > @@ -892,7 +872,8 @@ namespace gbe {
> > //for some functions, so we use global context now, need switch
> > to new context later.
> > llvm::Module * out_module;
> > llvm::LLVMContext* llvm_ctx = &llvm::getGlobalContext();
> > - if (buildModuleFromSource(clName.c_str(), &out_module, llvm_ctx,
> > dumpLLVMFileName, clOpt,
> > +
> > + if (buildModuleFromSource(source, &out_module, llvm_ctx,
> > + dumpLLVMFileName, clOpt,
> > stringSize, err, errSize)) {
> > // Now build the program from llvm
> > if (err != NULL) {
> > @@ -907,7 +888,6 @@ namespace gbe {
> > llvm::errs() << options;
> > } else
> > p = NULL;
> > - remove(clName.c_str());
> > releaseLLVMContextLock();
> > return p;
> > }
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list