On 12/23/2016 05:39 PM, Lionel Landwerlin wrote: > The specification section 9.4 says : > > When an application attempts to create many pipelines in a single > command, it is possible that some subset may fail creation. In that > case, the corresponding entries in the pPipelines output array will > be filled with VK_NULL_HANDLE values. If any pipeline fails > creation (for example, due to out of memory errors), the > vkCreate*Pipelines commands will return an error code. The > implementation will attempt to create all pipelines, and only > return VK_NULL_HANDLE values for those that actually failed. > > Fixes : > > > dEQP-VK.api.object_management.alloc_callback_fail_multiple.graphics_pipeline > > v2: C is hard let's go shopping (Lionel) > > Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com> > --- > src/intel/vulkan/genX_pipeline.c | 24 ++++++++++-------------- > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/src/intel/vulkan/genX_pipeline.c > b/src/intel/vulkan/genX_pipeline.c > index 845d0204d8..eb58f2368f 100644 > --- a/src/intel/vulkan/genX_pipeline.c > +++ b/src/intel/vulkan/genX_pipeline.c > @@ -1470,21 +1470,19 @@ VkResult genX(CreateGraphicsPipelines)( > VkResult result = VK_SUCCESS; > > unsigned i = 0; > - for (; i < count; i++) { > + for (; i < count && result == VK_SUCCESS; i++) { >
Now that you are at it, I would move the declaration and initialization of "i" into the for statement: for (unsigned i = 0; i < count && result == VK_SUCCESS; i++) { > result = genX(graphics_pipeline_create)(_device, > pipeline_cache, > &pCreateInfos[i], > pAllocator, &pPipelines[i]); > if (result != VK_SUCCESS) { > - for (unsigned j = 0; j < i; j++) { > - anv_DestroyPipeline(_device, pPipelines[j], pAllocator); > - } > - > - return result; > + for (; i < count; i++) > + pPipelines[i] = VK_NULL_HANDLE; > + break; > } > } > > - return VK_SUCCESS; > + return result; > } > I know it was the previous behavior too, but from the spec quotation above it is not clear that once a pipeline creation fails, no more pipelines should be attempted creation. Actually, it explicitly says that "The implementation will attempt to create all pipelines", which is not the case here. I'm thinking on e.g, a momentary out-of-memory condition. At least I would add a comment saying that it will bail out upon the first error and why. > VkResult genX(CreateComputePipelines)( > @@ -1500,18 +1498,16 @@ VkResult genX(CreateComputePipelines)( > VkResult result = VK_SUCCESS; > > unsigned i = 0; > - for (; i < count; i++) { > + for (; i < count && result == VK_SUCCESS; i++) { > result = compute_pipeline_create(_device, pipeline_cache, > &pCreateInfos[i], > pAllocator, &pPipelines[i]); > if (result != VK_SUCCESS) { > - for (unsigned j = 0; j < i; j++) { > - anv_DestroyPipeline(_device, pPipelines[j], pAllocator); > - } > - > - return result; > + for (; i < count; i++) > + pPipelines[i] = VK_NULL_HANDLE; > + break; > } > } > > - return VK_SUCCESS; > + return result; > } Both comments apply here too. > -- > 2.11.0 > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev