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:[email protected]] On Behalf Of > Gong, Zhigang > Sent: Tuesday, September 8, 2015 14:33 > To: Luo, Xionghu; [email protected] > 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; [email protected] > > 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:[email protected]] On Behalf > > Of Zhigang Gong > > Sent: Monday, August 31, 2015 2:30 PM > > To: [email protected] > > 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 <[email protected]> > > --- > > 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 > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/beignet > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
