[Mesa-dev] [PATCH 7/7] clover: Sign-extend and zero-extend kernel arguments when required v2
Aaron Watry
awatry at gmail.com
Wed Jul 17 08:25:31 PDT 2013
On Tue, Jul 9, 2013 at 11:21 PM, Tom Stellard <tom at stellard.net> wrote:
> From: Tom Stellard <thomas.stellard at amd.com>
>
> v2:
> - Extend to target size rather than aligned size
> - Support for big-endian
> ---
> src/gallium/state_trackers/clover/core/kernel.cpp | 58 ++++++++++++++++------
> src/gallium/state_trackers/clover/core/kernel.hpp | 17 ++++---
> src/gallium/state_trackers/clover/core/module.hpp | 27 ++++++++--
> .../state_trackers/clover/llvm/invocation.cpp | 20 +++++++-
> 4 files changed, 95 insertions(+), 27 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp b/src/gallium/state_trackers/clover/core/kernel.cpp
> index 2b6fbe5..99723ff 100644
> --- a/src/gallium/state_trackers/clover/core/kernel.cpp
> +++ b/src/gallium/state_trackers/clover/core/kernel.cpp
> @@ -152,10 +152,12 @@ _cl_kernel::exec_context::bind(clover::command_queue *__q) {
> for (const clover::module::argument &arg : kern.module_args(*q)) {
> if (arg.type == clover::module::argument::pad) {
> input.resize(input.size() + arg.size);
> - } else {
> - assert(arg.arg_index >= 0);
> - kern.args[arg.arg_index]->bind(*this);
> + break;
My C++ is a bit weak, but this feels like it should be a continue
instead of a break. Not positive of the complete context this is
being called within, so feel free to tell me to go hide under my rock.
If this is supposed to bind the full list of kernel arguments and the
first argument requires padding/extension, I don't see how the
following arguments would get bound.
--Aaron
> }
> +
> + assert(arg.arg_index >= 0);
> +
> + kern.args[arg.arg_index]->bind(*this, arg);
> }
>
> // Create a new compute state if anything changed.
> @@ -202,9 +204,7 @@ _cl_kernel::argument::storage() const {
> return 0;
> }
>
> -_cl_kernel::scalar_argument::scalar_argument(size_t size) :
> - argument(size) {
> -}
> +_cl_kernel::scalar_argument::scalar_argument(size_t size) : argument(size) { }
>
> void
> _cl_kernel::scalar_argument::set(size_t size, const void *value) {
> @@ -216,8 +216,32 @@ _cl_kernel::scalar_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::scalar_argument::bind(exec_context &ctx) {
> - ctx.input.insert(ctx.input.end(), v.begin(), v.end());
> +_cl_kernel::scalar_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> + // Extend the value
> + bool little_endian = ctx.q->dev.endianness() == PIPE_ENDIAN_LITTLE;
> + bool has_sign;
> + if (little_endian) {
> + has_sign = v[__size - 1] & 0x80;
> + } else {
> + has_sign = v[0] & 0x80;
> + }
> + uint8_t ext_value;
> + if (arg.ext_type == module::argument::sext && has_sign) {
> + ext_value = 0xff;
> + } else {
> + ext_value = 0;
> + }
> +
> + if (little_endian) {
> + ctx.input.insert(ctx.input.end(), v.begin(), v.end());
> + }
> + for (unsigned i = __size; i < arg.target_size; ++i) {
> + ctx.input.push_back(ext_value);
> + }
> + if (!little_endian) {
> + ctx.input.insert(ctx.input.end(), v.begin(), v.end());
> + }
> }
>
> void
> @@ -241,7 +265,8 @@ _cl_kernel::global_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::global_argument::bind(exec_context &ctx) {
> +_cl_kernel::global_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> size_t offset = ctx.input.size();
> size_t idx = ctx.g_buffers.size();
>
> @@ -277,7 +302,8 @@ _cl_kernel::local_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::local_argument::bind(exec_context &ctx) {
> +_cl_kernel::local_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> size_t offset = ctx.input.size();
> size_t ptr = ctx.mem_local;
>
> @@ -308,7 +334,8 @@ _cl_kernel::constant_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::constant_argument::bind(exec_context &ctx) {
> +_cl_kernel::constant_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> size_t offset = ctx.input.size();
> size_t idx = ctx.resources.size();
>
> @@ -341,7 +368,8 @@ _cl_kernel::image_rd_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::image_rd_argument::bind(exec_context &ctx) {
> +_cl_kernel::image_rd_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> size_t offset = ctx.input.size();
> size_t idx = ctx.sviews.size();
>
> @@ -374,7 +402,8 @@ _cl_kernel::image_wr_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::image_wr_argument::bind(exec_context &ctx) {
> +_cl_kernel::image_wr_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> size_t offset = ctx.input.size();
> size_t idx = ctx.resources.size();
>
> @@ -404,7 +433,8 @@ _cl_kernel::sampler_argument::set(size_t size, const void *value) {
> }
>
> void
> -_cl_kernel::sampler_argument::bind(exec_context &ctx) {
> +_cl_kernel::sampler_argument::bind(exec_context &ctx,
> + const clover::module::argument &arg) {
> size_t idx = ctx.samplers.size();
>
> ctx.samplers.resize(idx + 1);
> diff --git a/src/gallium/state_trackers/clover/core/kernel.hpp b/src/gallium/state_trackers/clover/core/kernel.hpp
> index 2c540be..2ccd027 100644
> --- a/src/gallium/state_trackers/clover/core/kernel.hpp
> +++ b/src/gallium/state_trackers/clover/core/kernel.hpp
> @@ -84,7 +84,8 @@ public:
>
> /// Allocate the necessary resources to bind the specified
> /// object to this argument, and update \a ctx accordingly.
> - virtual void bind(exec_context &ctx) = 0;
> + virtual void bind(exec_context &ctx,
> + const clover::module::argument &arg) = 0;
>
> /// Free any resources that were allocated in bind().
> virtual void unbind(exec_context &ctx) = 0;
> @@ -125,7 +126,7 @@ private:
> scalar_argument(size_t size);
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> @@ -137,7 +138,7 @@ private:
> global_argument(size_t size);
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> @@ -151,7 +152,7 @@ private:
> virtual size_t storage() const;
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> @@ -163,7 +164,7 @@ private:
> constant_argument(size_t size);
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> @@ -176,7 +177,7 @@ private:
> image_rd_argument(size_t size);
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> @@ -189,7 +190,7 @@ private:
> image_wr_argument(size_t size);
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> @@ -202,7 +203,7 @@ private:
> sampler_argument(size_t size);
>
> virtual void set(size_t size, const void *value);
> - virtual void bind(exec_context &ctx);
> + virtual void bind(exec_context &ctx, const clover::module::argument &arg);
> virtual void unbind(exec_context &ctx);
>
> private:
> diff --git a/src/gallium/state_trackers/clover/core/module.hpp b/src/gallium/state_trackers/clover/core/module.hpp
> index d4b3413..c880315 100644
> --- a/src/gallium/state_trackers/clover/core/module.hpp
> +++ b/src/gallium/state_trackers/clover/core/module.hpp
> @@ -69,17 +69,36 @@ namespace clover {
> pad
> };
>
> + enum ext_type {
> + zext,
> + sext,
> + noext,
> + };
> +
> + argument(enum type type, size_t size, size_t arg_index,
> + size_t target_size, enum ext_type ext_type) : type(type),
> + size(size), arg_index(arg_index),
> + target_size(target_size), ext_type(ext_type) { }
> argument(enum type type, size_t size, size_t arg_index) : type(type),
> - size(size), arg_index(arg_index) { }
> - argument(enum type type, size_t size) : type(type), size(size),
> - arg_index(-1) { }
> - argument() : type(scalar), size(0), arg_index(-1) { }
> + size(size), arg_index(arg_index), target_size(size),
> + ext_type(noext) { }
> + argument(enum type type, size_t size) : type(type),
> + size(size), arg_index(-1), target_size(size),
> + ext_type(noext) { }
> + argument() : type(scalar), size(0), arg_index(-1), target_size(0),
> + ext_type(noext) { }
>
> type type;
> size_t size;
> /// The index of this argument in the function. If arg_index is -1,
> /// then this arguement is not a formal argument of the function.
> size_t arg_index;
> +
> + /// The size of this argument in the target architecture. This will
> + /// typically be the smallest natively supported integer size of a
> + /// target.
> + size_t target_size;
> + ext_type ext_type;
> };
>
> struct symbol {
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 68f7eac..16198e0 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -344,8 +344,26 @@ namespace {
> break;
> }
> } else {
> + unsigned attr_index = arg.getArgNo() + 1;
> + // Determine the argument's alignment. When the SExt or ZExt
> + // attribute is set on a function, it means that the caller must
> + // extend the value to the appropriate size. We will extend the
> + // value when binding it to a kernel.
> + // See _cl_kernel::scalar_argument::bind()
> + enum module::argument::ext_type ext_type =
> + module::argument::noext;
> +
> + if (kernel_func->getAttributes().hasAttribute(attr_index,
> + llvm::Attribute::SExt)) {
> + ext_type = module::argument::sext;
> + } else if (kernel_func->getAttributes().hasAttribute(attr_index,
> + llvm::Attribute::ZExt)) {
> + ext_type = module::argument::zext;
> + }
> +
> args.push_back(module::argument(module::argument::scalar,
> - arg_size, arg.getArgNo()));
> + arg_size, arg.getArgNo(),
> + target_size >> 3, ext_type));
> }
> if (aligned_size > target_size) {
> args.push_back(module::argument(module::argument::pad,
> --
> 1.7.11.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list