Sent a new patch with subject [Mesa-dev] [PATCH] clover: Pass image attributes to the kernel. <div class="gmail_quote">2015.07.06. 17:58 ezt írta ("Zoltán Gilián" <<a href="mailto:zoltan.gilian@gmail.com">zoltan.gilian@gmail.com</a>>): <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> 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 <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote: > Zoltán Gilián <<a href="mailto:zoltan.gilian@gmail.com">zoltan.gilian@gmail.com</a>> 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 <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote: >>> Zoltan Gilian <<a href="mailto:zoltan.gilian@gmail.com">zoltan.gilian@gmail.com</a>> 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 </blockquote></div>