[Mesa-dev] [PATCH v2 2/5] anv/descriptor_set: Use anv_multialloc for descriptor set layouts

[Nanley Chery]

On Fri, Apr 07, 2017 at 10:40:49AM -0700, Jason Ekstrand wrote:
> On Fri, Apr 7, 2017 at 1:14 AM, Chris Wilson <chris at chris-wilson.co.uk>
> wrote:
> 
> > On Thu, Apr 06, 2017 at 10:53:07PM -0700, Jason Ekstrand wrote:
> > > ---
> > >  src/intel/vulkan/anv_descriptor_set.c | 22 +++++++++++-----------
> > >  1 file changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_descriptor_set.c
> > b/src/intel/vulkan/anv_descriptor_set.c
> > > index 56a08ce..6f2280d 100644
> > > --- a/src/intel/vulkan/anv_descriptor_set.c
> > > +++ b/src/intel/vulkan/anv_descriptor_set.c
> > > @@ -42,7 +42,6 @@ VkResult anv_CreateDescriptorSetLayout(
> > >      VkDescriptorSetLayout*                      pSetLayout)
> > >  {
> > >     ANV_FROM_HANDLE(anv_device, device, _device);
> > > -   struct anv_descriptor_set_layout *set_layout;
> > >
> > >     assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_DESCRIPTOR_
> > SET_LAYOUT_CREATE_INFO);
> > >
> > > @@ -54,18 +53,19 @@ VkResult anv_CreateDescriptorSetLayout(
> > >           immutable_sampler_count += pCreateInfo->pBindings[j].
> > descriptorCount;
> > >     }
> > >
> > > -   size_t size = sizeof(struct anv_descriptor_set_layout) +
> > > -                 (max_binding + 1) * sizeof(set_layout->binding[0]) +
> > > -                 immutable_sampler_count * sizeof(struct anv_sampler *);
> > > +   struct anv_descriptor_set_layout *set_layout;
> > > +   struct anv_descriptor_set_binding_layout *bindings;
> > > +   struct anv_sampler **samplers;
> > >
> > > -   set_layout = vk_alloc2(&device->alloc, pAllocator, size, 8,
> > > -                           VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> > > -   if (!set_layout)
> > > -      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> > > +   struct anv_multialloc ma;
> > > +   anv_multialloc_init2(&ma, &device->alloc, pAllocator,
> > > +                        VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> > > +   anv_multialloc_add(&ma, &set_layout, 1);
> > > +   anv_multialloc_add(&ma, &bindings, max_binding + 1);
> > > +   anv_multialloc_add(&ma, &samplers, immutable_sampler_count);
> > >
> > > -   /* We just allocate all the samplers at the end of the struct */
> > > -   struct anv_sampler **samplers =
> > > -      (struct anv_sampler **)&set_layout->binding[max_binding + 1];
> > > +   if (!anv_multialloc_finish(&ma))
> > > +      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> >
> > As the outside observer, I suggest that using finish here is confusing.
> > Something like.
> >
> 
> Yeah, I didn't really like finsh either.
> 
> 
> >    ANV_MULTIALLOC(ma);
> >
> >    anv_multialloc_add(&ma, &set_layout, 1);
> >    anv_multialloc_add(&ma, &bindings, max_binding + 1);
> >    anv_multialloc_add(&ma, &samplers, immutable_sampler_count);
> >
> >    if (!anv_multialloc_alloc(&ma,
> >                              &device->alloc ?: pAllocator,
> >                              VK_SYSTEM_ALLOCATION_SCOPE_OBJECT))
> >       return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> >
> > fits your current vk_alloc pattern better?
> >
> 
> Perhaps.  I'm not sure whether I like putting the alloc and scope
> parameters early or late better.  I think I do like the ANV_MULTIALLOC(ma)
> declare+init helper.  Nanley?

I like this suggestion. It matches the fact that we only need the
allocator and scope when performing the allocation at the end.