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

[Francisco Jerez]

Francisco Jerez <currojerez at riseup.net> writes:

> Jan Vesely <jan.vesely at rutgers.edu> writes:
>
>> v2: use a new variable for aligned size
>>     add comment
>>     make both vars const
>>     only use the aligned value in argument constructors
>>     fix comment typo
>>
>> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
>
> Looks good to me, thanks,
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>

Thanks, pushed:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=7b11c97d31ea9ebdf5d4b89e60bcc96d256aa7a4

>> ---
>>  .../state_trackers/clover/llvm/invocation.cpp      | 23 +++++++++++++++-------
>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index a81bdf8..29d2986 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>
>> @@ -308,11 +309,19 @@ namespace {
>>  #endif
>>  
>>              llvm::Type *arg_type = arg.getType();
>> -            unsigned arg_size = TD.getTypeStoreSize(arg_type);
>> +            const unsigned arg_store_size = TD.getTypeStoreSize(arg_type);
>> +
>> +            // OCL specs Ch. 6.1.5: "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."
>> +            // We need this alignment for three element vectors, which
>> +            // can have non-power-of-2 size.
>> +            const unsigned arg_api_size =
>> +               util_next_power_of_two(arg_store_size);
>>  
>>              llvm::Type *target_type = arg_type->isIntegerTy() ?
>> -               TD.getSmallestLegalIntType(mod->getContext(), arg_size * 8) :
>> -               arg_type;
>> +               TD.getSmallestLegalIntType(mod->getContext(), arg_store_size * 8)
>> +               : arg_type;
>>              unsigned target_size = TD.getTypeStoreSize(target_type);
>>              unsigned target_align = TD.getABITypeAlignment(target_type);
>>  
>> @@ -326,19 +335,19 @@ namespace {
>>                 if (address_space == address_spaces[clang::LangAS::opencl_local
>>                                                       - clang::LangAS::Offset]) {
>>                    args.push_back(module::argument(module::argument::local,
>> -                                                  arg_size, target_size,
>> +                                                  arg_api_size, target_size,
>>                                                    target_align,
>>                                                    module::argument::zero_ext));
>>                 } else {
>>                    // XXX: Correctly handle constant address space.  There is no
>>                    // way for r600g to pass a handle for constant buffers back
>>                    // to clover like it can for global buffers, so
>> -                  // creating constant arguements will break r600g.  For now,
>> +                  // creating constant arguments will break r600g.  For now,
>>                    // continue treating constant buffers as global buffers
>>                    // until we can come up with a way to create handles for
>>                    // constant buffers.
>>                    args.push_back(module::argument(module::argument::global,
>> -                                                  arg_size, target_size,
>> +                                                  arg_api_size, target_size,
>>                                                    target_align,
>>                                                    module::argument::zero_ext));
>>                }
>> @@ -352,7 +361,7 @@ namespace {
>>                     module::argument::zero_ext);
>>  
>>                 args.push_back(
>> -                  module::argument(module::argument::scalar, arg_size,
>> +                  module::argument(module::argument::scalar, arg_api_size,
>>                                     target_size, target_align, ext_type));
>>              }
>>           }
>> -- 
>> 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/20140429/d0537d81/attachment.sig>