[Mesa-dev] [PATCH] clover: Implement image attribute getters

Zoltán Gilián zoltan.gilian at gmail.com
Fri Jul 10 02:25:33 PDT 2015


Sent a new patch with subject [Mesa-dev] [PATCH] clover: Pass image
attributes to the kernel.
2015.07.06. 17:58 ezt írta ("Zoltán Gilián" <zoltan.gilian at gmail.com>):

> > Hint: you'll need new
> > module::argument::semantic enums
>
> I see. Reworked it a bit.
>
> On Mon, Jul 6, 2015 at 1:13 PM, Francisco Jerez <currojerez at riseup.net>
> wrote:
> > Zoltán Gilián <zoltan.gilian at gmail.com> writes:
> >
> >>> This seems to be doing essentially the same thing as v1?  Is it the
> >>> right patch?
> >>
> >> The llvm pass was invoked in clover in v1. This patch relies on llvm
> >> to perform that task (). What this patch does basically is that it
> >> adds the image attributes to the end of the kernel input vector.
> >> The commit message of this patch is misleading, I'll fix it.
> >>
> > NAK.  Just like in v1, you're implementing the same pipe driver-specific
> > policy in Clover's core layer -- If you don't feel like fixing this
> > properly as I described in my reply to v1, it would be acceptable to
> > implement it for the time being using a workaround similar to
> > llvm/invocation.cpp:433 -- Hint: you'll need new
> > module::argument::semantic enums.
> >
> > Thanks.
> >
> >> On Wed, Jun 24, 2015 at 2:48 PM, Francisco Jerez <currojerez at riseup.net>
> wrote:
> >>> Zoltan Gilian <zoltan.gilian at gmail.com> writes:
> >>>
> >>>> Image attributes are passed to the kernel as hidden parameters after
> the
> >>>> image attribute itself. An llvm pass replaces the getter builtins to
> >>>> the appropriate parameters.
> >>>
> >>> This seems to be doing essentially the same thing as v1?  Is it the
> >>> right patch?
> >>>
> >>>> ---
> >>>>  src/gallium/state_trackers/clover/core/kernel.cpp  | 26 +++++++
> >>>>  src/gallium/state_trackers/clover/core/kernel.hpp  | 13 ++--
> >>>>  src/gallium/state_trackers/clover/core/memory.cpp  |  2 +-
> >>>>  .../state_trackers/clover/llvm/invocation.cpp      | 81
> +++++++++++++++++++++-
> >>>>  4 files changed, 116 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp
> b/src/gallium/state_trackers/clover/core/kernel.cpp
> >>>> index 0756f06..291c799 100644
> >>>> --- a/src/gallium/state_trackers/clover/core/kernel.cpp
> >>>> +++ b/src/gallium/state_trackers/clover/core/kernel.cpp
> >>>> @@ -185,6 +185,13 @@
> kernel::exec_context::bind(intrusive_ptr<command_queue> _q,
> >>>>        }
> >>>>     }
> >>>>
> >>>> +   // Bind image attribute args.
> >>>> +   for (const auto& arg: kern._args) {
> >>>> +      if (auto img_arg = dynamic_cast<image_argument*>(arg.get())) {
> >>>> +         img_arg->bind_attributes(*this);
> >>>> +      }
> >>>> +   }
> >>>> +
> >>>>     // Create a new compute state if anything changed.
> >>>>     if (!st || q != _q ||
> >>>>         cs.req_local_mem != mem_local ||
> >>>> @@ -465,6 +472,25 @@ kernel::constant_argument::unbind(exec_context
> &ctx) {
> >>>>  }
> >>>>
> >>>>  void
> >>>> +kernel::image_argument::bind_attributes(exec_context &ctx) {
> >>>> +   cl_image_format format = img->format();
> >>>> +   cl_uint attributes[] = {
> >>>> +         static_cast<cl_uint>(img->width()),
> >>>> +         static_cast<cl_uint>(img->height()),
> >>>> +         static_cast<cl_uint>(img->depth()),
> >>>> +         format.image_channel_data_type,
> >>>> +         format.image_channel_order};
> >>>> +   for (unsigned i = 0; i < 5; ++i) {
> >>>> +      auto v = bytes(attributes[i]);
> >>>> +
> >>>> +      extend(v, module::argument::zero_ext, sizeof(cl_uint));
> >>>> +      byteswap(v, ctx.q->device().endianness());
> >>>> +      align(ctx.input, sizeof(cl_uint));
> >>>> +      insert(ctx.input, v);
> >>>> +   }
> >>>> +}
> >>>> +
> >>>> +void
> >>>>  kernel::image_rd_argument::set(size_t size, const void *value) {
> >>>>     if (size != sizeof(cl_mem))
> >>>>        throw error(CL_INVALID_ARG_SIZE);
> >>>> diff --git a/src/gallium/state_trackers/clover/core/kernel.hpp
> b/src/gallium/state_trackers/clover/core/kernel.hpp
> >>>> index d6432a4..8c15b2f 100644
> >>>> --- a/src/gallium/state_trackers/clover/core/kernel.hpp
> >>>> +++ b/src/gallium/state_trackers/clover/core/kernel.hpp
> >>>> @@ -190,7 +190,14 @@ namespace clover {
> >>>>           pipe_surface *st;
> >>>>        };
> >>>>
> >>>> -      class image_rd_argument : public argument {
> >>>> +      class image_argument : public argument {
> >>>> +      public:
> >>>> +         void bind_attributes(exec_context &ctx);
> >>>> +      protected:
> >>>> +         image *img;
> >>>> +      };
> >>>> +
> >>>> +      class image_rd_argument : public image_argument {
> >>>>        public:
> >>>>           virtual void set(size_t size, const void *value);
> >>>>           virtual void bind(exec_context &ctx,
> >>>> @@ -198,11 +205,10 @@ namespace clover {
> >>>>           virtual void unbind(exec_context &ctx);
> >>>>
> >>>>        private:
> >>>> -         image *img;
> >>>>           pipe_sampler_view *st;
> >>>>        };
> >>>>
> >>>> -      class image_wr_argument : public argument {
> >>>> +      class image_wr_argument : public image_argument {
> >>>>        public:
> >>>>           virtual void set(size_t size, const void *value);
> >>>>           virtual void bind(exec_context &ctx,
> >>>> @@ -210,7 +216,6 @@ namespace clover {
> >>>>           virtual void unbind(exec_context &ctx);
> >>>>
> >>>>        private:
> >>>> -         image *img;
> >>>>           pipe_surface *st;
> >>>>        };
> >>>>
> >>>> diff --git a/src/gallium/state_trackers/clover/core/memory.cpp
> b/src/gallium/state_trackers/clover/core/memory.cpp
> >>>> index 055336a..b852e68 100644
> >>>> --- a/src/gallium/state_trackers/clover/core/memory.cpp
> >>>> +++ b/src/gallium/state_trackers/clover/core/memory.cpp
> >>>> @@ -189,7 +189,7 @@ image2d::image2d(clover::context &ctx,
> cl_mem_flags flags,
> >>>>                   const cl_image_format *format, size_t width,
> >>>>                   size_t height, size_t row_pitch,
> >>>>                   void *host_ptr) :
> >>>> -   image(ctx, flags, format, width, height, 0,
> >>>> +   image(ctx, flags, format, width, height, 1,
> >>>>           row_pitch, 0, height * row_pitch, host_ptr) {
> >>>>  }
> >>>>
> >>>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>>> index 9b91fee..a33d450 100644
> >>>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>>> @@ -80,6 +80,7 @@
> >>>>  using namespace clover;
> >>>>
> >>>>  namespace {
> >>>> +
> >>>>  #if 0
> >>>>     void
> >>>>     build_binary(const std::string &source, const std::string &target,
> >>>> @@ -340,17 +341,65 @@ namespace {
> >>>>        PM.run(*mod);
> >>>>     }
> >>>>
> >>>> +   const llvm::MDNode *
> >>>> +   get_kernel_metadata(const llvm::Function *kernel_func) {
> >>>> +      auto mod = kernel_func->getParent();
> >>>> +      auto kernels_node = mod->getNamedMetadata("opencl.kernels");
> >>>> +      if (!kernels_node) {
> >>>> +         return nullptr;
> >>>> +      }
> >>>> +
> >>>> +      const llvm::MDNode *kernel_node = nullptr;
> >>>> +      for (unsigned i = 0; i < kernels_node->getNumOperands(); ++i) {
> >>>> +#if HAVE_LLVM >= 0x0306
> >>>> +         auto func = llvm::mdconst::dyn_extract<llvm::Function>(
> >>>> +#else
> >>>> +         auto func = llvm::dyn_cast<llvm::Function>(
> >>>> +#endif
> >>>> +
> kernels_node->getOperand(i)->getOperand(0));
> >>>> +         if (func == kernel_func) {
> >>>> +            kernel_node = kernels_node->getOperand(i);
> >>>> +            break;
> >>>> +         }
> >>>> +      }
> >>>> +
> >>>> +      return kernel_node;
> >>>> +   }
> >>>> +
> >>>> +   std::vector<llvm::StringRef>
> >>>> +   get_kernel_access_qualifiers(const llvm::Function *kernel_func) {
> >>>> +      auto num_args = kernel_func->getArgumentList().size();
> >>>> +      auto access_quals = std::vector<llvm::StringRef>(num_args);
> >>>> +
> >>>> +      auto kernel_node = get_kernel_metadata(kernel_func);
> >>>> +      auto aq_node =
> llvm::cast<llvm::MDNode>(kernel_node->getOperand(2));
> >>>> +      auto str_node =
> llvm::cast<llvm::MDString>(aq_node->getOperand(0));
> >>>> +      assert(str_node->getString() == "kernel_arg_access_qual" &&
> >>>> +             "Cannot find kernel_arg_access_qual metadata node.");
> >>>> +      assert(aq_node->getNumOperands() == num_args + 1 &&
> >>>> +             "Wrong number of operands in kernel_arg_access_qual
> metadata.");
> >>>> +
> >>>> +      for (unsigned i = 1; i < aq_node->getNumOperands(); ++i) {
> >>>> +        auto aq = llvm::cast<llvm::MDString>(aq_node->getOperand(i));
> >>>> +        access_quals[i-1] = aq->getString();
> >>>> +      }
> >>>> +
> >>>> +      return access_quals;
> >>>> +   }
> >>>> +
> >>>>     std::vector<module::argument>
> >>>>     get_kernel_args(const llvm::Module *mod, const std::string
> &kernel_name,
> >>>>                     const clang::LangAS::Map &address_spaces) {
> >>>>
> >>>>        std::vector<module::argument> args;
> >>>>        llvm::Function *kernel_func = mod->getFunction(kernel_name);
> >>>> +      auto access_quals = get_kernel_access_qualifiers(kernel_func);
> >>>>
> >>>>        llvm::DataLayout TD(mod);
> >>>>
> >>>> +      unsigned arg_idx = 0;
> >>>>        for (llvm::Function::const_arg_iterator I =
> kernel_func->arg_begin(),
> >>>> -                                      E = kernel_func->arg_end(); I
> != E; ++I) {
> >>>> +                           E = kernel_func->arg_end(); I != E; ++I,
> ++arg_idx) {
> >>>>           const llvm::Argument &arg = *I;
> >>>>
> >>>>           llvm::Type *arg_type = arg.getType();
> >>>> @@ -375,6 +424,36 @@ namespace {
> >>>>           }
> >>>>
> >>>>           if (arg_type->isPointerTy()) {
> >>>> +            // XXX: Figure out LLVM->OpenCL address space mappings
> for each
> >>>> +            // target.  I think we need to ask clang what these
> are.  For now,
> >>>> +            // pretend everything is in the global address space.
> >>>> +            llvm::Type *elem_type =
> arg_type->getPointerElementType();
> >>>> +            if (elem_type->isStructTy()) {
> >>>> +               llvm::StringRef type_name =
> elem_type->getStructName();
> >>>> +               llvm::StringRef access_qual = access_quals[arg_idx];
> >>>> +
> >>>> +               typename module::argument::type marg_type;
> >>>> +               if (type_name.startswith("opencl.image2d_t")) {
> >>>> +                  if (access_qual == "write_only") {
> >>>> +                     marg_type = module::argument::image2d_wr;
> >>>> +                  } else {
> >>>> +                     marg_type = module::argument::image2d_rd;
> >>>> +                  }
> >>>> +               } else if (type_name.startswith("opencl.image3d_t")) {
> >>>> +                  if (access_qual == "write_only") {
> >>>> +                     marg_type = module::argument::image3d_wr;
> >>>> +                  } else {
> >>>> +                     marg_type = module::argument::image3d_rd;
> >>>> +                  }
> >>>> +               } else {
> >>>> +                  continue;
> >>>> +               }
> >>>> +
> >>>> +               args.push_back(module::argument(marg_type,
> >>>> +                              arg_store_size, target_size,
> target_align,
> >>>> +                              module::argument::zero_ext));
> >>>> +               continue;
> >>>> +            }
> >>>>              unsigned address_space =
> llvm::cast<llvm::PointerType>(arg_type)->getAddressSpace();
> >>>>              if (address_space ==
> address_spaces[clang::LangAS::opencl_local
> >>>>                                                       -
> clang::LangAS::Offset]) {
> >>>> --
> >>>> 2.4.2
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150710/47090372/attachment-0001.html>


More information about the mesa-dev mailing list