[Mesa-dev] [PATCH 3/3] clover: add clCompile

EdB edb+mesa at sigluy.net
Fri Oct 10 11:00:32 PDT 2014


On Friday 10 October 2014 10:16:08 Tom Stellard wrote:
> On Thu, Oct 09, 2014 at 09:22:46PM +0200, EdB wrote:
> > On Thursday, October 09, 2014 06:29:40 AM Tom Stellard wrote:
> > > On Sun, Sep 28, 2014 at 12:57:22PM +0200, EdB wrote:
> > > > ---
> > > > 
> > > >  src/gallium/state_trackers/clover/api/dispatch.cpp |  2 +-
> > > >  src/gallium/state_trackers/clover/api/program.cpp  | 39
> > > >  +++++++++++++++++++--- .../state_trackers/clover/core/compiler.hpp
> > > >  
> > > >   | 12 ++++---
> > > >  
> > > >  src/gallium/state_trackers/clover/core/error.hpp   |  2 +-
> > > >  src/gallium/state_trackers/clover/core/program.cpp | 14 ++++++--
> > > >  src/gallium/state_trackers/clover/core/program.hpp |  5 ++-
> > > >  .../state_trackers/clover/llvm/invocation.cpp      | 39
> > > >  ++++++++++++++++++---- 7 files changed, 93 insertions(+), 20
> > > >  deletions(-)
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/api/dispatch.cpp
> > > > b/src/gallium/state_trackers/clover/api/dispatch.cpp index
> > > > 35d150d..b5a4094 100644
> > > > --- a/src/gallium/state_trackers/clover/api/dispatch.cpp
> > > > +++ b/src/gallium/state_trackers/clover/api/dispatch.cpp
> > > > @@ -122,7 +122,7 @@ namespace clover {
> > > > 
> > > >        clReleaseDevice,
> > > >        clCreateImage,
> > > >        clCreateProgramWithBuiltInKernels,
> > > > 
> > > > -      NULL, // clCompileProgram
> > > > +      clCompileProgram,
> > > > 
> > > >        NULL, // clLinkProgram
> > > >        clUnloadPlatformCompiler,
> > > >        NULL, // clGetKernelArgInfo
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/api/program.cpp
> > > > b/src/gallium/state_trackers/clover/api/program.cpp index
> > > > 6771735..33df0cd 100644
> > > > --- a/src/gallium/state_trackers/clover/api/program.cpp
> > > > +++ b/src/gallium/state_trackers/clover/api/program.cpp
> > > > @@ -152,14 +152,34 @@ CLOVER_API cl_int
> > > > 
> > > >  clBuildProgram(cl_program d_prog, cl_uint num_devs,
> > > >  
> > > >                 const cl_device_id *d_devs, const char *p_opts,
> > > >                 void (*pfn_notify)(cl_program, void *),
> > > > 
> > > > -               void *user_data) try {
> > > > +               void *user_data) {
> > > > +   cl_int error = clCompileProgram(d_prog, num_devs, d_devs, p_opts,
> > > > +                                   0, 0, 0,
> > > > +                                   pfn_notify, user_data);
> > > > +   return error == CL_COMPILE_PROGRAM_FAILURE ?
> > > > +             CL_BUILD_PROGRAM_FAILURE : error;
> > > > +}
> > > > +
> > > > +CLOVER_API cl_int
> > > > +clCompileProgram(cl_program d_prog, cl_uint num_devs,
> > > > +                 const cl_device_id *d_devs, const char *p_opts,
> > > > +                 cl_uint num_headers, const cl_program
> > > > *d_header_progs,
> > > > +                 const char **header_names,
> > > > +                 void (*pfn_notify)(cl_program, void *),
> > > > +                 void *user_data) try {
> > > > 
> > > >     auto &prog = obj(d_prog);
> > > >     
> > > >     auto devs = (d_devs ? objs(d_devs, num_devs) :
> > > >                  ref_vector<device>(prog.context().devices()));
> > > >     
> > > >     auto opts = (p_opts ? p_opts : "");
> > > > 
> > > > -   if (bool(num_devs) != bool(d_devs) ||
> > > > -       (!pfn_notify && user_data))
> > > > +   if (bool(num_devs) != bool(d_devs))
> > > > +      throw error(CL_INVALID_VALUE);
> > > > +
> > > > +   if (!pfn_notify && user_data)
> > > > +      throw error(CL_INVALID_VALUE);
> > > > +
> > > > +   if (bool(num_headers) != bool(header_names) ||
> > > > +       bool(num_headers) != bool(d_header_progs))
> > > > 
> > > >        throw error(CL_INVALID_VALUE);
> > > >     
> > > >     if (any_of([&](const device &dev) {
> > > > 
> > > > @@ -170,7 +190,18 @@ clBuildProgram(cl_program d_prog, cl_uint
> > > > num_devs,
> > > > 
> > > >     if (prog.kernel_ref_count())
> > > >     
> > > >        throw error(CL_INVALID_OPERATION);
> > > > 
> > > > -   prog.build(devs, opts);
> > > > +   std::map<const std::string, const std::string> headers;
> > > > +   for (cl_uint i = 0; i < num_headers; ++i) {
> > > > +      auto h_name = std::string(header_names[i]);
> > > > +      auto &h_prog = obj(d_header_progs[i]);
> > > > +
> > > > +      if (!h_prog.has_source)
> > > > +         throw error(CL_INVALID_OPERATION);
> > > > +
> > > > +      headers.insert(make_pair(h_name, h_prog.source()));
> > > > +   }
> > > > +
> > > > +   prog.build(devs, opts, headers);
> > > > 
> > > >     return CL_SUCCESS;
> > > >  
> > > >  } catch (error &e) {
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/core/compiler.hpp
> > > > b/src/gallium/state_trackers/clover/core/compiler.hpp index
> > > > 6ef84d1..c2c4063 100644
> > > > --- a/src/gallium/state_trackers/clover/core/compiler.hpp
> > > > +++ b/src/gallium/state_trackers/clover/core/compiler.hpp
> > > > @@ -29,11 +29,15 @@
> > > > 
> > > >  #include "pipe/p_defines.h"
> > > >  
> > > >  namespace clover {
> > > > 
> > > > +   typedef compat::pair<compat::vector_ref<const char>,
> > > > +                compat::vector_ref<const char> > vector_ref_pair;
> > > > +
> > > > 
> > > >     module compile_program_llvm(const compat::string &source,
> > > > 
> > > > -                               pipe_shader_ir ir,
> > > > -                               const compat::string &target,
> > > > -                               const compat::string &opts,
> > > > -                               compat::string &r_log);
> > > > +                   const compat::vector<vector_ref_pair> &headers,
> > > > +                   pipe_shader_ir ir,
> > > > +                   const compat::string &target,
> > > > +                   const compat::string &opts,
> > > > +                   compat::string &r_log);
> > > > 
> > > >     module compile_program_tgsi(const compat::string &source);
> > > >  
> > > >  }
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/core/error.hpp
> > > > b/src/gallium/state_trackers/clover/core/error.hpp index
> > > > cecbe9b..7b010f1
> > > > 100644
> > > > --- a/src/gallium/state_trackers/clover/core/error.hpp
> > > > +++ b/src/gallium/state_trackers/clover/core/error.hpp
> > > > @@ -67,7 +67,7 @@ namespace clover {
> > > > 
> > > >     class build_error : public error {
> > > >     
> > > >     public:
> > > >        build_error(const compat::string &what = "") :
> > > > -         error(CL_BUILD_PROGRAM_FAILURE, what) {
> > > > +         error(CL_COMPILE_PROGRAM_FAILURE, what) {
> > > > 
> > > >        }
> > > >     
> > > >     };
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/core/program.cpp
> > > > b/src/gallium/state_trackers/clover/core/program.cpp index
> > > > 6c224db..0940697 100644
> > > > --- a/src/gallium/state_trackers/clover/core/program.cpp
> > > > +++ b/src/gallium/state_trackers/clover/core/program.cpp
> > > > @@ -41,7 +41,8 @@ program::program(clover::context &ctx,
> > > > 
> > > >  }
> > > >  
> > > >  void
> > > > 
> > > > -program::build(const ref_vector<device> &devs, const char *opts) {
> > > > +program::build(const ref_vector<device> &devs, const char *opts,
> > > > +               const string_map &headers) {
> > > > 
> > > >     if (has_source) {
> > > >     
> > > >        _devices = devs;
> > > > 
> > > > @@ -54,10 +55,19 @@ program::build(const ref_vector<device> &devs,
> > > > const
> > > > char *opts) {>
> > > > 
> > > >           compat::string log;
> > > > 
> > > > +         compat::vector<vector_ref_pair> cpt_headers;
> > > > +         for (auto &header : headers) {
> > > > +            vector_ref_pair h;
> > > > +            h.first = header.first;
> > > > +            h.second = header.second;
> > > > +            cpt_headers.push_back(h);
> > > > +         }
> > > > +
> > > > 
> > > >           try {
> > > >           
> > > >              auto module = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
> > > >              
> > > >                             compile_program_tgsi(_source) :
> > > > -                           compile_program_llvm(_source,
> > > > dev.ir_format(),
> > > > +                           compile_program_llvm(_source, cpt_headers,
> > > > +                                                dev.ir_format(),
> > > > 
> > > >                                                  dev.ir_target(),
> > > >                                                  build_opts(dev),
> > > >                                                  log));
> > > >              
> > > >              _binaries.insert({ &dev, module });
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/core/program.hpp
> > > > b/src/gallium/state_trackers/clover/core/program.hpp index
> > > > 4bb5b68..8e05075 100644
> > > > --- a/src/gallium/state_trackers/clover/core/program.hpp
> > > > +++ b/src/gallium/state_trackers/clover/core/program.hpp
> > > > @@ -29,6 +29,8 @@
> > > > 
> > > >  #include "core/context.hpp"
> > > >  #include "core/module.hpp"
> > > > 
> > > > +typedef std::map<const std::string, const std::string> string_map;
> > > > +
> > > > 
> > > >  namespace clover {
> > > >  
> > > >     class program : public ref_counter, public _cl_program {
> > > > 
> > > >     private:
> > > > @@ -46,7 +48,8 @@ namespace clover {
> > > > 
> > > >        program &
> > > >        operator=(const program &prog) = delete;
> > > > 
> > > > -      void build(const ref_vector<device> &devs, const char *opts);
> > > > +      void build(const ref_vector<device> &devs, const char *opts,
> > > > +                 const string_map &headers);
> > > > 
> > > >        const bool has_source;
> > > >        const std::string &source() const;
> > > > 
> > > > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > > > b/src/gallium/state_trackers/clover/llvm/invocation.cpp index
> > > > 7bca0d6..9f16adc 100644
> > > > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > > > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > > > @@ -118,6 +118,7 @@ namespace {
> > > > 
> > > >     llvm::Module *
> > > >     compile(llvm::LLVMContext &llvm_ctx, const std::string &source,
> > > > 
> > > > +           const compat::vector<vector_ref_pair> &headers,
> > > > 
> > > >             const std::string &name, const std::string &triple,
> > > >             const std::string &processor, const std::string &opts,
> > > >             clang::LangAS::Map& address_spaces, compat::string &r_log)
> > > >             {
> > > > 
> > > > @@ -213,12 +214,35 @@ namespace {
> > > > 
> > > >  #if HAVE_LLVM >= 0x0306
> > > >  
> > > >        c.getPreprocessorOpts().addRemappedFile(name,
> > > > 
> > > > -
> > > > llvm::MemoryBuffer::getMemBuffer(source).release()); +
> > > > 
> > > >                   llvm::MemoryBuffer::getMemBuffer(source).release());
> > > 
> > > Unnecessary whitespace change.
> > > 
> > > >  #else
> > > >  
> > > >        c.getPreprocessorOpts().addRemappedFile(name,
> > > >        
> > > >                                        llvm::MemoryBuffer::getMemBuffe
> > > >                                        r(so
> > > >                                        urce));
> > > >  
> > > >  #endif
> > > > 
> > > > +      if (headers.size()) {
> > > > +         const std::string tmp_header_path = "/tmp/clover/";
> > > > +
> > > 
> > > Why are we using a /tmp/clover here?  Aren't the headers stored in
> > > memory?
> > 
> > Yes they are, but unless I'm wrong, they need a valid path. It could have
> > been /in_memory/clover for exemple (I hesitate)
> > If you think of a better path, it will be ok, but better avoid using ./
> 
> I'm just confused, because it looks like we are adding a file
> located in /tmp/clover/headername.h to the compile.  If the full
> file is stored in memory, then I think we should be able to use
> addRemappedFile() without adding a header search path.
> 
If I didn't add this new path, the files are not found.
For exemple when doing

c.getPreprocessorOpts().addRemappedFile("headername.h",
llvm::MemoryBuffer::getMemBuffer(the_buffer)

clang fails to compile.
However it will find a headername.h if it exists in "./" and in this special 
case, the real one is used (I guess it's a bug).

Currently clang allow you to include a file in the current dir and I thought it 
would be strange to include those temporary buffer from the current dir to 
avoid in confusion

EdB

> -Tom
> 
> > EdB
> > 
> > > -Tom
> > > 
> > > > +         c.getHeaderSearchOpts().AddPath(tmp_header_path,
> > > > +                                         clang::frontend::Angled,
> > > > +                                         false, false
> > > > +                                         #if HAVE_LLVM < 0x0303
> > > > +                                         , false
> > > > +                                         #endif
> > > > +                                         );
> > > > +
> > > > +        for (size_t i = 0; i < headers.size(); ++i) {
> > > > +           vector_ref_pair header = headers[i];
> > > > +           const std::string path = tmp_header_path +
> > > > std::string(header.first.begin()); +
> > > > c.getPreprocessorOpts().addRemappedFile(path,
> > > > +#if HAVE_LLVM >= 0x0306
> > > > +
> > > > llvm::MemoryBuffer::getMemBuffer(header.second.begin()).release());
> > > > +#else
> > > > +
> > > > llvm::MemoryBuffer::getMemBuffer(header.second.begin())); +#endif
> > > > +        }
> > > > +      }
> > > > +
> > > > 
> > > >        // Setting this attribute tells clang to link this file before
> > > >        // performing any optimizations.  This is required so that
> > > >        // we can replace calls to the OpenCL C barrier() builtin
> > > > 
> > > > @@ -401,10 +425,11 @@ namespace {
> > > > 
> > > >  module
> > > >  clover::compile_program_llvm(const compat::string &source,
> > > > 
> > > > -                             enum pipe_shader_ir ir,
> > > > -                             const compat::string &target,
> > > > -                             const compat::string &opts,
> > > > -                             compat::string &r_log) {
> > > > +                  const compat::vector<vector_ref_pair> &headers,
> > > > +                  enum pipe_shader_ir ir,
> > > > +                  const compat::string &target,
> > > > +                  const compat::string &opts,
> > > > +                  compat::string &r_log) {
> > > > 
> > > >     std::vector<llvm::Function *> kernels;
> > > >     size_t processor_str_len =
> > > >     std::string(target.begin()).find_first_of("-");
> > > > 
> > > > @@ -417,8 +442,8 @@ clover::compile_program_llvm(const compat::string
> > > > &source,>
> > > > 
> > > >     // The input file name must have the .cl extension in order for
> > > >     the
> > > >     // CompilerInvocation class to recognize it as an OpenCL source
> > > >     file.
> > > > 
> > > > -   llvm::Module *mod = compile(llvm_ctx, source, "input.cl", triple,
> > > > processor, -                               opts, address_spaces,
> > > > r_log);
> > > > +   llvm::Module *mod = compile(llvm_ctx, source, headers, "input.cl",
> > > > +                               triple, processor, opts,
> > > > address_spaces,
> > > > r_log);>
> > > > 
> > > >     find_kernels(mod, kernels);



More information about the mesa-dev mailing list