[Mesa-dev] [PATCH 1/1] clover: Align kernel argument sizes to nearest power of 2

[Francisco Jerez]

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

> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> ---
>
> Hi,
>
> this is the alternative (power of 2) approach to hadling kernel args,
> as discussed in this thread:
> http://lists.freedesktop.org/archives/mesa-dev/2014-April/thread.html#58209
>
> Jan
>
>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index a81bdf8..8e5b49b 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -64,6 +64,7 @@
>  
>  #include "pipe/p_state.h"
>  #include "util/u_memory.h"
> +#include "util/u_math.h"
>  
>  #include <iostream>
>  #include <iomanip>
> @@ -309,6 +310,9 @@ namespace {
>  
>              llvm::Type *arg_type = arg.getType();
>              unsigned arg_size = TD.getTypeStoreSize(arg_type);
> +            if (!util_is_power_of_two(arg_size)) {

The 'if' is redundant, util_next_power_of_two() does nothing when its
argument is a power of two already.  Could you add a comment here
quoting the relevant text from the spec (Section 6.1.5 of the OpenCL 1.2
specification, "A built-in data type that is not a power of two bytes in
size must be aligned to the next larger power of two.") and mentioning
that in particular this is required to give 3-element vectors the
correct API size?

> +              arg_size = util_next_power_of_two(arg_size);

Could you define a new variable for this (e.g. "const unsigned
arg_api_size"), and maybe rename "arg_size" to "arg_store_size" so the
difference is clear?  I believe we'll want to use "arg_store_size" in
line 314, and "arg_api_size" in lines 329, 341 and 355.

Thank you.

> +            }
>  
>              llvm::Type *target_type = arg_type->isIntegerTy() ?
>                 TD.getSmallestLegalIntType(mod->getContext(), arg_size * 8) :
> -- 
> 1.9.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140425/77b51afe/attachment.sig>