[Beignet] [PATCH] GBE: Use addRemappedFile to avoid creating temporary cl source file.
Gong, Zhigang
zhigang.gong at intel.com
Tue Sep 8 01:27:36 PDT 2015
> -----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