[Mesa-dev] [PATCH v4 6/6] radeonsi: try to re-use previously deleted bindless descriptor slots

[Samuel Pitoiset]

On 08/23/2017 07:30 AM, Timothy Arceri wrote:
> This is causing piglit regressions for me. For example:
> 
> ./bin/shader_runner 
> tests/spec/arb_bindless_texture/execution/images/multiple-resident-images-reading.shader_test 
> -auto -fb
> 
> Unexpected GL error: GL_OUT_OF_MEMORY 0x505
> (Error at 
> /home/tarceri/git/Mesa_arrays_of_arrays_piglit/tests/shaders/shader_runner.c:3560) 
> 
> glMakeImageHandleResidentARB error
> PIGLIT: {"result": "fail" }

Mmh, you are right, but I can only reproduce with a non-debug build, weird.

> 
> 
> On 22/08/17 07:22, Marek Olšák wrote:
>> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>>
>> So let's commit this!
>>
>> Marek
>>
>> On Mon, Aug 21, 2017 at 4:50 PM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>> Currently, when the array is full it is resized but it can grow
>>> over and over because we don't try to re-use descriptor slots.
>>>
>>> v4: - rebase on top of idalloc changes
>>> v3: - use new idalloc gallium module
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>>   src/gallium/drivers/radeonsi/si_descriptors.c | 36 
>>> +++++++++++++++++++++------
>>>   src/gallium/drivers/radeonsi/si_pipe.h        |  2 ++
>>>   2 files changed, 31 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
>>> b/src/gallium/drivers/radeonsi/si_descriptors.c
>>> index f14fce103f..c869dac9bb 100644
>>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>>> @@ -61,6 +61,7 @@
>>>   #include "gfx9d.h"
>>>
>>>   #include "util/hash_table.h"
>>> +#include "util/u_idalloc.h"
>>>   #include "util/u_format.h"
>>>   #include "util/u_memory.h"
>>>   #include "util/u_upload_mgr.h"
>>> @@ -2335,23 +2336,27 @@ static void 
>>> si_init_bindless_descriptors(struct si_context *sctx,
>>>           * considered to be a valid handle.
>>>           */
>>>          sctx->num_bindless_descriptors = 1;
>>> +
>>> +       /* Track which bindless slots are used (or not). */
>>> +       util_idalloc_init(&sctx->bindless_used_slots);
>>> +       util_idalloc_resize(&sctx->bindless_used_slots, num_elements);
>>> +
>>> +       /* Reserve slot 0 because it's an invalid handle for 
>>> bindless. */
>>> +       assert(!util_idalloc_alloc(&sctx->bindless_used_slots));
>>>   }
>>>
>>>   static void si_release_bindless_descriptors(struct si_context *sctx)
>>>   {
>>>          si_release_descriptors(&sctx->bindless_descriptors);
>>> +       util_idalloc_fini(&sctx->bindless_used_slots);
>>>   }
>>>
>>> -static unsigned
>>> -si_create_bindless_descriptor(struct si_context *sctx, uint32_t 
>>> *desc_list,
>>> -                             unsigned size)
>>> +static unsigned si_get_first_free_bindless_slot(struct si_context 
>>> *sctx)
>>>   {
>>>          struct si_descriptors *desc = &sctx->bindless_descriptors;
>>> -       unsigned desc_slot, desc_slot_offset;
>>> -
>>> -       /* Reserve a new slot for this bindless descriptor. */
>>> -       desc_slot = sctx->num_bindless_descriptors++;
>>> +       unsigned desc_slot;
>>>
>>> +       desc_slot = util_idalloc_alloc(&sctx->bindless_used_slots);
>>>          if (desc_slot >= desc->num_elements) {
>>>                  /* The array of bindless descriptors is full, resize 
>>> it. */
>>>                  unsigned slot_size = desc->element_dw_size * 4;
>>> @@ -2363,6 +2368,20 @@ si_create_bindless_descriptor(struct 
>>> si_context *sctx, uint32_t *desc_list,
>>>                  desc->num_active_slots = new_num_elements;
>>>          }
>>>
>>> +       assert(desc_slot);
>>> +       return desc_slot;
>>> +}
>>> +
>>> +static unsigned
>>> +si_create_bindless_descriptor(struct si_context *sctx, uint32_t 
>>> *desc_list,
>>> +                             unsigned size)
>>> +{
>>> +       struct si_descriptors *desc = &sctx->bindless_descriptors;
>>> +       unsigned desc_slot, desc_slot_offset;
>>> +
>>> +       /* Find a free slot. */
>>> +       desc_slot = si_get_first_free_bindless_slot(sctx);
>>> +
>>>          /* For simplicity, sampler and image bindless descriptors 
>>> use fixed
>>>           * 16-dword slots for now. Image descriptors only need 
>>> 8-dword but this
>>>           * doesn't really matter because no real apps use image 
>>> handles.
>>> @@ -2475,6 +2494,9 @@ static void si_delete_texture_handle(struct 
>>> pipe_context *ctx, uint64_t handle)
>>>
>>>          tex_handle = (struct si_texture_handle *)entry->data;
>>>
>>> +       /* Allow this descriptor slot to be re-used. */
>>> +       util_idalloc_free(&sctx->bindless_used_slots, 
>>> tex_handle->desc_slot);
>>> +
>>>          pipe_sampler_view_reference(&tex_handle->view, NULL);
>>>          _mesa_hash_table_remove(sctx->tex_handles, entry);
>>>          FREE(tex_handle);
>>> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
>>> b/src/gallium/drivers/radeonsi/si_pipe.h
>>> index 56c3b08188..8a475d3b05 100644
>>> --- a/src/gallium/drivers/radeonsi/si_pipe.h
>>> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
>>> @@ -29,6 +29,7 @@
>>>   #include "si_shader.h"
>>>
>>>   #include "util/u_dynarray.h"
>>> +#include "util/u_idalloc.h"
>>>
>>>   #ifdef PIPE_ARCH_BIG_ENDIAN
>>>   #define SI_BIG_ENDIAN 1
>>> @@ -430,6 +431,7 @@ struct si_context {
>>>
>>>          /* Bindless descriptors. */
>>>          struct si_descriptors   bindless_descriptors;
>>> +       struct util_idalloc     bindless_used_slots;
>>>          unsigned                num_bindless_descriptors;
>>>          bool                    bindless_descriptors_dirty;
>>>          bool                    graphics_bindless_pointer_dirty;
>>> -- 
>>> 2.14.1
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>