[Mesa-dev] [PATCH 0/3] cl workdim v2

[Francisco Jerez]

Jan Vesely <jan.vesely at rutgers.edu> writes:

> On Wed, 2014-10-08 at 18:02 +0300, Francisco Jerez wrote:
>> Jan Vesely <jan.vesely at rutgers.edu> writes:
>> 
>> > [SNIP]
>> >> > 
>> >> > > I also don't like that this way there is no difference between
>> >> > > explicit and implicit kernel arguments. On the other hand it's simple,
>> >> > > and does not need additional per driver code.
>> >> > >
>> >> > Yeah...  We definitely want to hide these from the user, as e.g. the
>> >> > CL_KERNEL_NUM_ARGS param is required by the spec to return the number of
>> >> > arguments provided by the user, and we don't want the user to set
>> >> > implicit args, so it gets a bit messy.  I think I like better your
>> >> > original idea of passing them as launch_grid() arguments, even though
>> >> > the grid offset and dimension parameters are somewhat artificial from a
>> >> > the hardware's point of view.
>> >> 
>> >> sorry to bug you some more with this. I tried one more thing before
>> >> going back to the launch_grid parameters. this time it implements a
>> >> parallel infrastructure for implicit arguments by creating artificial
>> >> module arguments for uint and size_t (I don't think we need more for
>> >> implicit arguments).
>> >> 
>> >> I only added the work dimension argument but adding more should be easy.
>> >> If you think that the launch_grid way is better, I'll stop experimenting
>> >> as I ran out of ideas I wanted to try.
>> >
>> > ping
>> > should I just resend using git instead of attachments?
>> 
>> Hi Jan, I'm sorry, I finally had a while to have a look into this.  I've
>> taken your series and tried to fix the couple of issues I wasn't very
>> comfortable with, see the attached series.  Does it look OK to you?
>> Note that it's completely untested, maybe you could give it a run on
>> your system?
>
> Hi,
>
> It took me a while to get back to this too.
>
> the first patch is kind of unrelated and imo can go in independently
> (you can add my R-b).
>
Thanks, just pushed it with your R-b.

> I'll need to spend some more time (hopefully this weekend) to fully
> understand the rest and give it a R-b (if you need/want it).

Please do.

> but it works (with the same changes to llvm and libclc as my patches
> need), with the attached fix.

Oh, good catch, thanks.

> so with that change you can add my acked/tested by.
> I ran a full piglit with no changes compared to my version
>
> regards,
> Jan
>
>
>> 
>> Thanks.
>> 
>> >
>> >> 
>> >> thanks,
>> >> jan
>> >
>> > [SNIP]
>> >
>> > -- 
>> > Jan Vesely <jan.vesely at rutgers.edu>
>> 
>
> -- 
> Jan Vesely <jan.vesely at rutgers.edu>
> diff --git a/src/gallium/state_trackers/clover/core/module.hpp b/src/gallium/state_trackers/clover/core/module.hpp
> index 268e3ba..ee6caf9 100644
> --- a/src/gallium/state_trackers/clover/core/module.hpp
> +++ b/src/gallium/state_trackers/clover/core/module.hpp
> @@ -80,7 +80,7 @@ namespace clover {
>                    enum semantic semantic = general) :
>              type(type), size(size),
>              target_size(target_size), target_align(target_align),
> -            ext_type(ext_type), semantic(general) { }
> +            ext_type(ext_type), semantic(semantic) { }
>  
>           argument(enum type type, size_t size) :
>              type(type), size(size),
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141011/105ea0e7/attachment-0001.sig>