Zoltán Gilián <zoltan.gil...@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 <curroje...@riseup.net> > wrote: >> Zoltan Gilian <zoltan.gil...@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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev