v3dv: don't leak NIR code in pipelines

The pipeline stages have a reference to the NIR code produced from
the SPIR-V shader modules, but they never destroy it.

It should also be noted that our coordinate shader stage was sharing
the NIR with the vertex shader stage, which is kind of tricky to handle
and probably very error prone. Just make sure each pipeline stage has
owns it NIR shader and that we always free it when the stage is
destroyed.

Also, for the case of NIR modules created by the driver internally,
we always need to clone them, since the driver will destroy the NIR
as soon as it is done creating pipelines with it. We could also not
clone it and let the pipeline stage take ownership of the NIR code for
NIR modules, but that would be inconsistent with how ownership works for
SPIR-V modules.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6766>
This commit is contained in:
Iago Toral Quiroga 2020-04-23 09:59:05 +02:00 committed by Marge Bot
parent c414a241d0
commit 77bccfd828

View file

@ -103,6 +103,8 @@ destroy_pipeline_stage(struct v3dv_device *device,
}
}
ralloc_free(p_stage->nir);
_mesa_hash_table_destroy(p_stage->cache, NULL);
vk_free2(&device->alloc, pAllocator, p_stage);
@ -365,7 +367,13 @@ shader_module_compile_to_nir(struct v3dv_device *device,
nir_validate_shader(nir, "after spirv_to_nir");
free(spec_entries);
} else {
nir = stage->module->nir;
/* For NIR modules created by the driver we can't consume the NIR
* directly, we need to clone it first, since ownership of the NIR code
* (as with SPIR-V code for SPIR-V shaders), belongs to the creator
* of the module and modules can be destroyed immediately after been used
* to create pipelines.
*/
nir = nir_shader_clone(NULL, stage->module->nir);
nir_validate_shader(nir, "nir module");
}
assert(nir->info.stage == stage->stage);
@ -1052,7 +1060,7 @@ pipeline_stage_create_vs_bin(const struct v3dv_pipeline_stage *src,
p_stage->stage = src->stage;
p_stage->entrypoint = src->entrypoint;
p_stage->module = src->module;
p_stage->nir = src->nir;
p_stage->nir = nir_shader_clone(NULL, src->nir);
/* Technically we could share the hash_table, but having their own makes
* destroy p_stage more straightforward
@ -1403,8 +1411,14 @@ pipeline_compile_graphics(struct v3dv_pipeline *pipeline,
*/
assert(pipeline->fs);
pipeline->vs = p_stage;
/* Make sure we do all our common lowering *before* we create the vs
* and vs_bin pipeline stages, since from that point forward we need to
* run lowerings for both of them separately, since each stage will
* own its NIR code.
*/
lower_vs_io(p_stage->nir);
pipeline->vs = p_stage;
pipeline->vs_bin = pipeline_stage_create_vs_bin(pipeline->vs, pAllocator);
/* FIXME: likely this to be moved to a gather info method to a full
@ -1414,8 +1428,6 @@ pipeline_compile_graphics(struct v3dv_pipeline *pipeline,
pCreateInfo->pInputAssemblyState;
pipeline->vs->topology = vk_to_pipe_prim_type[ia_info->topology];
lower_vs_io(p_stage->nir);
/* Note that at this point we would compile twice, one for vs and
* other for vs_bin. For now we are maintaining two pipeline_stages.
*