> 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 <curroje...@riseup.net> wrote:
> 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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
