[Mesa-dev] [PATCH 2/2] clover: fix getting struct args api size

[Francisco Jerez]

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

> On Mon, 2016-06-13 at 17:24 -0700, Francisco Jerez wrote:
>> Serge Martin <edb+mesa at sigluy.net> writes:
>> 
>> > This fix getting the size of a struct arg. vec3 types still work
>> > ok.
>> > Only buit-in args need to have power of two alignment,
>> > getTypeAllocSize
>> > reports the correct size.
>> > ---
>> >  src/gallium/state_trackers/clover/llvm/invocation.cpp | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > index 03487d6..9af51539 100644
>> > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > @@ -472,7 +472,8 @@ namespace {
>> >           // aligned to the next larger power of two".  We need
>> > this
>> >           // alignment for three element vectors, which have
>> >           // non-power-of-2 store size.
>> > -         const unsigned arg_api_size =
>> > util_next_power_of_two(arg_store_size);
>> > +         const unsigned arg_api_size = arg_type->isStructTy() ?
>> > +               arg_store_size :
>> > util_next_power_of_two(arg_store_size);
>> >  
>> Hm...  Isn't this still going to be broken if you pass a struct
>> argument
>> to a kernel function and the alignment of any of the struct members
>> doesn't match the target-specific data layout?  Not sure we can fix
>> this
>> sensibly without requiring the target's data layout to match the CL
>> API
>> exactly.  Any suggestions Tom?
>
> according to 6.7.2.1 compilers can arbitrarily insert padding between
> struct members (except at the beginning).

What spec version are you looking at?  My CL spec doesn't have any
section labeled 6.7.2.1.

> Even if size/alignment of individual members match CL API exactly,
> there's no guarantee that the structure layout/size will be the same.
>
How can you exchange structured data with a CL kernel then, assuming
that the layout of structure types in memory is fully unspecified as you
say?

> Jan
>
>> 
>> >           llvm::Type *target_type = arg_type->isIntegerTy() ?
>> >                 TD.getSmallestLegalIntType(mod->getContext(),
>> > arg_store_size * 8)
>> > -- 
>> > 2.5.5
> -- 
>
> Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160622/84674276/attachment-0001.sig>