[Mesa-dev] [PATCH 7/7] clover: Sign-extend and zero-extend kernel arguments when required v2

Tom Stellard tom at stellard.net
Wed Jul 17 14:19:55 PDT 2013


On Wed, Jul 17, 2013 at 10:25:31AM -0500, Aaron Watry wrote:
> 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.
> 

Good catch, that is definitely wrong.  I will fix it.

-Tom

> >        }
> > +
> > +      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