> -----Original Message----- > From: Yang, Rong R > Sent: Tuesday, September 8, 2015 4:11 PM > To: Gong, Zhigang; Luo, Xionghu; beignet@lists.freedesktop.org > Subject: RE: [Beignet] [PATCH] GBE: Use addRemappedFile to avoid creating > temporary cl source file. > > Is this remapped file virtual file? If it is not a virtual file, I am afraid > it is not > thread/process safe.
It's not a file, just a map belongs to the clang::CompilerInvocation object. > > > -----Original Message----- > > From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf > > Of Gong, Zhigang > > Sent: Tuesday, September 8, 2015 14:33 > > To: Luo, Xionghu; beignet@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@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-boun...@lists.freedesktop.org] On > > > Behalf Of Zhigang Gong > > > Sent: Monday, August 31, 2015 2:30 PM > > > To: beignet@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.g...@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@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/beignet > > _______________________________________________ > > Beignet mailing list > > Beignet@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet