[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:39:41 PDT 2015
Ok, pushed. Thanks.
> -----Original Message-----
> From: Gong, Zhigang
> Sent: Tuesday, September 8, 2015 16:28
> To: Yang, Rong R; Luo, Xionghu; beignet at lists.freedesktop.org
> Subject: RE: [Beignet] [PATCH] GBE: Use addRemappedFile to avoid creating
> temporary cl source file.
>
>
>
> > -----Original Message-----
> > From: Yang, Rong R
> > Sent: Tuesday, September 8, 2015 4:11 PM
> > To: Gong, Zhigang; Luo, Xionghu; beignet at 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-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