On Mon, 2015-08-03 at 13:38 +0200, Zoltán Gilián wrote:
> Committed as 9ef5b7a.
> 
> On Fri, Jul 31, 2015 at 4:07 PM, Zoltán Gilián <zoltan.gil...@gmail.com> 
> wrote:
> > Could you please commit this?
> >
> > On Mon, Jul 27, 2015 at 1:28 PM, Francisco Jerez <curroje...@riseup.net> 
> > wrote:
> >> Zoltan Gilian <zoltan.gil...@gmail.com> writes:
> >>
> >>> Read-only and write-only image arguments are recognized and
> >>> distinguished.
> >>> Attributes of the image arguments are passed to the kernel as implicit
> >>> arguments.
> >>> ---
> >>>  src/gallium/state_trackers/clover/core/kernel.cpp  |  28 +++++
> >>>  src/gallium/state_trackers/clover/core/kernel.hpp  |  15 ++-
> >>>  src/gallium/state_trackers/clover/core/module.hpp  |   4 +-
> >>>  .../state_trackers/clover/llvm/invocation.cpp      | 135 
> >>> ++++++++++++++++++++-
> >>>  4 files changed, 171 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp 
> >>> b/src/gallium/state_trackers/clover/core/kernel.cpp
> >>> index 0756f06..955ff7b 100644
> >>> --- a/src/gallium/state_trackers/clover/core/kernel.cpp
> >>> +++ b/src/gallium/state_trackers/clover/core/kernel.cpp
> >>> @@ -182,6 +182,34 @@ 
> >>> kernel::exec_context::bind(intrusive_ptr<command_queue> _q,
> >>>           }
> >>>           break;
> >>>        }
> >>> +      case module::argument::image_size: {
> >>> +         auto img = dynamic_cast<image_argument &>(**(explicit_arg - 
> >>> 1)).get();
> >>> +         std::vector<cl_uint> image_size{
> >>> +               static_cast<cl_uint>(img->width()),
> >>> +               static_cast<cl_uint>(img->height()),
> >>> +               static_cast<cl_uint>(img->depth())};
> >>> +         for (auto x : image_size) {
> >>> +            auto arg = argument::create(marg);
> >>> +
> >>> +            arg->set(sizeof(x), &x);
> >>> +            arg->bind(*this, marg);
> >>> +         }
> >>> +         break;
> >>> +      }
> >>> +      case module::argument::image_format: {
> >>> +         auto img = dynamic_cast<image_argument &>(**(explicit_arg - 
> >>> 1)).get();
> >>> +         cl_image_format fmt = img->format();
> >>> +         std::vector<cl_uint> image_format{
> >>> +               static_cast<cl_uint>(fmt.image_channel_data_type),
> >>> +               static_cast<cl_uint>(fmt.image_channel_order)};
> >>> +         for (auto x : image_format) {
> >>> +            auto arg = argument::create(marg);
> >>> +
> >>> +            arg->set(sizeof(x), &x);
> >>> +            arg->bind(*this, marg);
> >>> +         }
> >>> +         break;
> >>> +      }
> >>>        }
> >>>     }
> >>>
> >>> diff --git a/src/gallium/state_trackers/clover/core/kernel.hpp 
> >>> b/src/gallium/state_trackers/clover/core/kernel.hpp
> >>> index d6432a4..4ba6ff4 100644
> >>> --- a/src/gallium/state_trackers/clover/core/kernel.hpp
> >>> +++ b/src/gallium/state_trackers/clover/core/kernel.hpp
> >>> @@ -190,7 +190,16 @@ namespace clover {
> >>>           pipe_surface *st;
> >>>        };
> >>>
> >>> -      class image_rd_argument : public argument {
> >>> +      class image_argument : public argument {
> >>> +      public:
> >>> +         const image *get() const {
> >>> +            return img;
> >>> +         }
> >>> +      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 +207,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 +218,6 @@ namespace clover {
> >>>           virtual void unbind(exec_context &ctx);
> >>>
> >>>        private:
> >>> -         image *img;
> >>>           pipe_surface *st;
> >>>        };
> >>>
> >>> diff --git a/src/gallium/state_trackers/clover/core/module.hpp 
> >>> b/src/gallium/state_trackers/clover/core/module.hpp
> >>> index 9d65688..5db0548 100644
> >>> --- a/src/gallium/state_trackers/clover/core/module.hpp
> >>> +++ b/src/gallium/state_trackers/clover/core/module.hpp
> >>> @@ -72,7 +72,9 @@ namespace clover {
> >>>           enum semantic {
> >>>              general,
> >>>              grid_dimension,
> >>> -            grid_offset
> >>> +            grid_offset,
> >>> +            image_size,
> >>> +            image_format
> >>>           };
> >>>
> >>>           argument(enum type type, size_t size,
> >>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
> >>> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> index 924cb36..86859af 100644
> >>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> @@ -344,18 +344,91 @@ namespace {
> >>>        PM.run(*mod);
> >>>     }
> >>>
> >>> +   // Kernel metadata
> >>> +
> >>> +   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;
> >>> +   }
> >>> +
> >>> +   llvm::MDNode*
> >>> +   node_from_op_checked(const llvm::MDOperand &md_operand,
> >>> +                        llvm::StringRef expect_name,
> >>> +                        unsigned expect_num_args)
> >>> +   {
> >>> +      auto node = llvm::cast<llvm::MDNode>(md_operand);
> >>> +      assert(node->getNumOperands() == expect_num_args &&
> >>> +             "Wrong number of operands.");
> >>> +
> >>> +      auto str_node = llvm::cast<llvm::MDString>(node->getOperand(0));
> >>> +      assert(str_node->getString() == expect_name &&
> >>> +             "Wrong metadata node name.");
> >>> +
> >>> +      return node;
> >>> +   }

This part breaks build with llvm 3.5:
llvm/invocation.cpp:375:37: error: ‘MDOperand’ in namespace ‘llvm’ does not 
name a type
    node_from_op_checked(const llvm::MDOperand &md_operand,
                                     ^
In file included from /usr/include/llvm/IR/Module.h:24:0,
                 from llvm/invocation.cpp:37:
/usr/include/llvm/IR/Metadata.h: In function ‘llvm::MDNode* 
{anonymous}::node_from_op_checked(const int&, llvm::StringRef, unsigned int)’:
/usr/include/llvm/IR/Metadata.h:75:3: error: ‘llvm::MDNode::MDNode(const 
llvm::MDNode&)’ is private
   MDNode(const MDNode &) LLVM_DELETED_FUNCTION;
   ^
llvm/invocation.cpp:379:54: error: within this context
       auto node = llvm::cast<llvm::MDNode>(md_operand);
                                                      ^
llvm/invocation.cpp:379:54: error: use of deleted function 
‘llvm::MDNode::MDNode(const llvm::MDNode&)’
In file included from /usr/include/llvm/IR/Module.h:24:0,
                 from llvm/invocation.cpp:37:
/usr/include/llvm/IR/Metadata.h:75:3: note: declared here
   MDNode(const MDNode &) LLVM_DELETED_FUNCTION;
   ^
/usr/include/llvm/IR/Metadata.h:114:3: error: ‘virtual llvm::MDNode::~MDNode()’ 
is private
   ~MDNode();
   ^
llvm/invocation.cpp:379:54: error: within this context
       auto node = llvm::cast<llvm::MDNode>(md_operand);
                                                      ^
In file included from /usr/include/llvm/IR/Module.h:24:0,
                 from llvm/invocation.cpp:37:
/usr/include/llvm/IR/Metadata.h:114:3: error: ‘virtual llvm::MDNode::~MDNode()’ 
is private
   ~MDNode();
   ^
llvm/invocation.cpp:379:54: error: within this context
       auto node = llvm::cast<llvm::MDNode>(md_operand);
                                                      ^
llvm/invocation.cpp:383:54: error: base operand of ‘->’ has non-pointer type 
‘llvm::MDNode’
       auto str_node = llvm::cast<llvm::MDString>(node->getOperand(0));
                                                      ^
llvm/invocation.cpp:387:14: error: cannot convert ‘llvm::MDNode’ to 
‘llvm::MDNode*’ in return
       return node;
              ^
llvm/invocation.cpp: In function ‘std::vector<{anonymous}::kernel_arg_md> 
{anonymous}::get_kernel_arg_md(const llvm::Function*)’:
llvm/invocation.cpp:402:63: error: invalid conversion from ‘llvm::Value*’ to 
‘int’ [-fpermissive]
       auto aq = node_from_op_checked(kernel_node->getOperand(2),
                                                               ^
llvm/invocation.cpp:375:4: note: initializing argument 1 of ‘llvm::MDNode* 
{anonymous}::node_from_op_checked(const int&, llvm::StringRef, unsigned int)’
    node_from_op_checked(const llvm::MDOperand &md_operand,
    ^
llvm/invocation.cpp:404:63: error: invalid conversion from ‘llvm::Value*’ to 
‘int’ [-fpermissive]
       auto ty = node_from_op_checked(kernel_node->getOperand(3),
                                                               ^
llvm/invocation.cpp:375:4: note: initializing argument 1 of ‘llvm::MDNode* 
{anonymous}::node_from_op_checked(const int&, llvm::StringRef, unsigned int)’
    node_from_op_checked(const llvm::MDOperand &md_operand,
    ^
llvm/invocation.cpp: In function ‘llvm::MDNode* 
{anonymous}::node_from_op_checked(const int&, llvm::StringRef, unsigned int)’:
llvm/invocation.cpp:388:4: warning: control reaches end of non-void function 
[-Wreturn-type]
    }
    ^

Jan

> >>> +
> >>> +   struct kernel_arg_md {
> >>> +      llvm::StringRef type_name;
> >>> +      llvm::StringRef access_qual;
> >>> +      kernel_arg_md(llvm::StringRef type_name_, llvm::StringRef 
> >>> access_qual_):
> >>> +         type_name(type_name_), access_qual(access_qual_) {}
> >>> +   };
> >>> +
> >>> +   std::vector<kernel_arg_md>
> >>> +   get_kernel_arg_md(const llvm::Function *kernel_func) {
> >>> +      auto num_args = kernel_func->getArgumentList().size();
> >>> +
> >>> +      auto kernel_node = get_kernel_metadata(kernel_func);
> >>> +      auto aq = node_from_op_checked(kernel_node->getOperand(2),
> >>> +                                     "kernel_arg_access_qual", num_args 
> >>> + 1);
> >>> +      auto ty = node_from_op_checked(kernel_node->getOperand(3),
> >>> +                                     "kernel_arg_type", num_args + 1);
> >>> +
> >>> +      std::vector<kernel_arg_md> res;
> >>> +      res.reserve(num_args);
> >>> +      for (unsigned i = 0; i < num_args; ++i) {
> >>> +         res.push_back(kernel_arg_md(
> >>> +            llvm::cast<llvm::MDString>(ty->getOperand(i+1))->getString(),
> >>> +            
> >>> llvm::cast<llvm::MDString>(aq->getOperand(i+1))->getString()));
> >>> +      }
> >>> +
> >>> +      return res;
> >>> +   }
> >>> +
> >>>     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);
> >>> +      assert(kernel_func && "Kernel name not found in module.");
> >>> +      auto arg_md = get_kernel_arg_md(kernel_func);
> >>>
> >>>        llvm::DataLayout TD(mod);
> >>> +      llvm::Type *size_type =
> >>> +         TD.getSmallestLegalIntType(mod->getContext(), sizeof(cl_uint) * 
> >>> 8);
> >>>
> >>> -      for (llvm::Function::const_arg_iterator I = 
> >>> kernel_func->arg_begin(),
> >>> -                                      E = kernel_func->arg_end(); I != 
> >>> E; ++I) {
> >>> -         const llvm::Argument &arg = *I;
> >>> +      for (const auto &arg: kernel_func->args()) {
> >>>
> >>>           llvm::Type *arg_type = arg.getType();
> >>>           const unsigned arg_store_size = TD.getTypeStoreSize(arg_type);
> >>> @@ -373,6 +446,59 @@ namespace {
> >>>           unsigned target_size = TD.getTypeStoreSize(target_type);
> >>>           unsigned target_align = TD.getABITypeAlignment(target_type);
> >>>
> >>> +         llvm::StringRef type_name = arg_md[arg.getArgNo()].type_name;
> >>> +         llvm::StringRef access_qual = 
> >>> arg_md[arg.getArgNo()].access_qual;
> >>> +
> >>> +         // Image
> >>> +         const bool is_image2d = type_name == "image2d_t";
> >>> +         const bool is_image3d = type_name == "image3d_t";
> >>> +         if (is_image2d || is_image3d) {
> >>> +            const bool is_write_only = access_qual == "write_only";
> >>> +            const bool is_read_only = access_qual == "read_only";
> >>> +
> >>> +            typename module::argument::type marg_type;
> >>> +            if (is_image2d && is_read_only) {
> >>> +               marg_type = module::argument::image2d_rd;
> >>> +            } else if (is_image2d && is_write_only) {
> >>> +               marg_type = module::argument::image2d_wr;
> >>> +            } else if (is_image3d && is_read_only) {
> >>> +               marg_type = module::argument::image3d_rd;
> >>> +            } else if (is_image3d && is_write_only) {
> >>> +               marg_type = module::argument::image3d_wr;
> >>> +            } else {
> >>> +               assert(0 && "Wrong image access qualifier");
> >>> +            }
> >>> +
> >>> +            args.push_back(module::argument(marg_type,
> >>> +                                            arg_store_size, target_size,
> >>> +                                            target_align,
> >>> +                                            module::argument::zero_ext));
> >>> +            continue;
> >>> +         }
> >>> +
> >>> +         // Image size implicit argument
> >>> +         if (type_name == "__llvm_image_size") {
> >>> +            args.push_back(module::argument(module::argument::scalar,
> >>> +                                            sizeof(cl_uint),
> >>> +                                            
> >>> TD.getTypeStoreSize(size_type),
> >>> +                                            
> >>> TD.getABITypeAlignment(size_type),
> >>> +                                            module::argument::zero_ext,
> >>> +                                            
> >>> module::argument::image_size));
> >>> +            continue;
> >>> +         }
> >>> +
> >>> +         // Image format implicit argument
> >>> +         if (type_name == "__llvm_image_format") {
> >>> +            args.push_back(module::argument(module::argument::scalar,
> >>> +                                            sizeof(cl_uint),
> >>> +                                            
> >>> TD.getTypeStoreSize(size_type),
> >>> +                                            
> >>> TD.getABITypeAlignment(size_type),
> >>> +                                            module::argument::zero_ext,
> >>> +                                            
> >>> module::argument::image_format));
> >>> +            continue;
> >>> +         }
> >>> +
> >>> +         // Other types
> >>>           if (llvm::isa<llvm::PointerType>(arg_type) && 
> >>> arg.hasByValAttr()) {
> >>>              arg_type =
> >>>                    
> >>> llvm::dyn_cast<llvm::PointerType>(arg_type)->getElementType();
> >>> @@ -417,9 +543,6 @@ namespace {
> >>>        // Append implicit arguments.  XXX - The types, ordering and
> >>>        // vector size of the implicit arguments should depend on the
> >>>        // target according to the selected calling convention.
> >>> -      llvm::Type *size_type =
> >>> -         TD.getSmallestLegalIntType(mod->getContext(), sizeof(cl_uint) * 
> >>> 8);
> >>> -
> >>>        args.push_back(
> >>>           module::argument(module::argument::scalar, sizeof(cl_uint),
> >>>                            TD.getTypeStoreSize(size_type),
> >>> --
> >>> 2.4.6
> >>
> >> Reviewed-by: Francisco Jerez <curroje...@riseup.net>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


-- 
Jan Vesely <jan.ves...@rutgers.edu>

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to