[Mesa-dev] [PATCH 1/7] isl/i965/fs: SSBO/UBO buffers need size padding if not multiple of 32-bit (v2)

[Chema Casanova]

On 26/02/18 15:40, Ilia Mirkin wrote:
> On Mon, Feb 26, 2018 at 9:14 AM, Jose Maria Casanova Crespo
> <jmcasanova at igalia.com> wrote:
>> The surfaces that backup the GPU buffers have a boundary check that
>> considers that access to partial dwords are considered out-of-bounds.
>> For example, buffers with 1/3 16-bit elemnts has size 2 or 6 and the
>> last two bytes would always be read as 0 or its writting ignored.
>>
>> The introduction of 16-bit types implies that we need to align the size
>> to 4-bytes multiples so that partial dwords could be read/written.
>> Adding an inconditional +2 size to buffers not being multiple of 2
>> solves this issue for the general cases of UBO or SSBO.
>>
>> But, when unsized arrays of 16-bit elements are used it is not possible
>> to know if the size was padded or not. To solve this issue the
>> implementation calculates the needed size of the buffer surfaces,
>> as suggested by Jason:
>>
>> surface_size = 2 * aling_u64(buffer_size, 4)  - buffer_size
>>
>> So when we calculate backwards the buffer_size in the backend we
>> update the resinfo return value with:
>>
>> buffer_size = (surface_size & ~3) - (surface_size & 3)
>>
>> It is also exposed this buffer requirements when robust buffer access
>> is enabled so these buffer sizes recommend being multiple of 4.
>>
>> v2: (Jason Ekstrand)
>>     Move padding logic fron anv to isl_surface_state
>>     Move calculus of original size from spirv to driver backend
>> ---
>>  src/intel/compiler/brw_fs_nir.cpp | 27 ++++++++++++++++++++++++++-
>>  src/intel/isl/isl_surface_state.c | 21 ++++++++++++++++++++-
>>  src/intel/vulkan/anv_device.c     | 11 +++++++++++
>>  3 files changed, 57 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp
>> index 8efec34cc9d..d017af040b4 100644
>> --- a/src/intel/compiler/brw_fs_nir.cpp
>> +++ b/src/intel/compiler/brw_fs_nir.cpp
>> @@ -4290,7 +4290,32 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>>        inst->mlen = 1;
>>        inst->size_written = 4 * REG_SIZE;
>>
>> -      bld.MOV(retype(dest, ret_payload.type), component(ret_payload, 0));
>> +      /* SKL PRM, vol07, 3D Media GPGPU Engine, Bounds Checking and Faulting:
>> +       *
>> +       * "Out-of-bounds checking is always performed at a DWord granularity. If
>> +       * any part of the DWord is out-of-bounds then the whole DWord is
>> +       * considered out-of-bounds."
>> +       *
>> +       * This implies that types with size smaller than 4-bytes (16-bits) need
> 
> 32 bits?

The 16-bits was a kind of example, of type with size less than 4-bytes,
so better remove it.

Thanks

Chema

>> +       * to be padded if they don't complete the last dword of the buffer. But
>> +       * as we need to maintain the original size we need to reverse the padding
>> +       * calculation to return the correct size to know the  number of elements
>> +       * of an unsized array. As we stored in the last two bits of the size
>> +       * of the buffer the needed padding we calculate here:
>> +       *
>> +       * buffer_size = resinfo_size & ~3 - resinfo_size & 3
>> +       */
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>