[Mesa-dev] [PATCH v5 02/70] i965: Use 16-byte offset alignment for shader storage buffers

[Samuel Iglesias Gonsálvez]

On 23/09/15 02:28, Ben Widawsky wrote:
> On Tue, Sep 15, 2015 at 11:37:22AM -0700, Kristian Høgsberg wrote:
>> On Thu, Sep 10, 2015 at 03:35:18PM +0200, Iago Toral Quiroga wrote:
>>> This is the same we do for other things like uniforms because it ensures
>>> optimal performance.
>>>
>>> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_context.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
>>> index 907b2a0..c8e8a68 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>>> @@ -558,6 +558,7 @@ brw_initialize_context_constants(struct brw_context *brw)
>>>      * However, unaligned accesses are slower, so enforce buffer alignment.
>>>      */
>>>     ctx->Const.UniformBufferOffsetAlignment = 16;
>>> +   ctx->Const.ShaderStorageBufferOffsetAlignment = 16;
>>
>> This should be a cacheline (64 bytes) so that we can safely have the
>> CPU and GPU writing the same SSBO on non-cachecoherent systems (our
>> Atom CPUs).  With UBOs, the GPU never writes, so there's no
>> problem. For an SSBO, the GPU and the CPU can be updating disjoint
>> regions of the buffer simultaneously and that will break if the
>> regions overlap the same cacheline.
>>
>> With that change,
>>
>> Reviewed-by: Kristian Høgsberg <krh at bitplanet.net>
>>
> 
> I know nothing about SSBOs...
> 
> I agree with Kristian assuming something in the API disallows simultaneous
> access to the same region from CPU and GPU - in which case, cacheline alignment
> doesn't buy anything. Also, when you make the change, please add a comment about
> why its 64.
> 

It is already added in the commit log, but as you think it should be
added as a comment in the code, I will do it too.

Thanks,

Sam

>>>     ctx->Const.TextureBufferOffsetAlignment = 16;
>>>     ctx->Const.MaxTextureBufferSize = 128 * 1024 * 1024;
>>>  
>>> -- 
>>> 1.9.1
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>