> -----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
