[Mesa-dev] [PATCH 07/16] radeonsi: add reference count to si_compute

[Gert Wollny]

Hello Nicolai, 

I've spotted a little problem in you code (see below). 

Best, 
Gert 


Am Mittwoch, den 16.08.2017, 13:05 +0200 schrieb Nicolai Hähnle:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> To allow keep-alive for deferred logging.
> ---
>  src/gallium/drivers/radeonsi/si_compute.c | 24 ++++++++++++++-------
> ---
>  src/gallium/drivers/radeonsi/si_compute.h | 14 ++++++++++++++
>  2 files changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_compute.c
> b/src/gallium/drivers/radeonsi/si_compute.c
> index 5efdd3919d2..8c016ba65ab 100644
> --- a/src/gallium/drivers/radeonsi/si_compute.c
> +++ b/src/gallium/drivers/radeonsi/si_compute.c
> @@ -151,6 +151,7 @@ static void *si_create_compute_state(
>  	struct si_screen *sscreen = (struct si_screen *)ctx->screen;
>  	struct si_compute *program = CALLOC_STRUCT(si_compute);
>  
> +	pipe_reference_init(&program->reference, 1);
>  	program->screen = (struct si_screen *)ctx->screen;
>  	program->ir_type = cso->ir_type;
>  	program->local_size = cso->req_local_mem;
> @@ -855,20 +856,24 @@ static void si_launch_grid(
>  		sctx->b.flags |= SI_CONTEXT_CS_PARTIAL_FLUSH;
>  }
>  
> +void si_destroy_compute(struct si_compute *program)
> +{
> +	if (program->ir_type == PIPE_SHADER_IR_TGSI) {
> +		util_queue_drop_job(&program->screen-
> >shader_compiler_queue,
> +				    &program->ready);
> +		util_queue_fence_destroy(&program->ready);
> +	}
> +
> +	si_shader_destroy(&program->shader);
> +	FREE(program);
> +}
>  
>  static void si_delete_compute_state(struct pipe_context *ctx, void*
> state){
>  	struct si_compute *program = (struct si_compute *)state;
>  	struct si_context *sctx = (struct si_context*)ctx;
>  
> -	if (!state) {
> +	if (!state)
>  		return;
> -	}
> -
> -	if (program->ir_type == PIPE_SHADER_IR_TGSI) {
> -		util_queue_drop_job(&sctx->screen-
> >shader_compiler_queue,
> -				    &program->ready);
> -		util_queue_fence_destroy(&program->ready);
> -	}
>  
>  	if (program == sctx->cs_shader_state.program)
>  		sctx->cs_shader_state.program = NULL;
> @@ -876,8 +881,7 @@ static void si_delete_compute_state(struct
> pipe_context *ctx, void* state){
>  	if (program == sctx->cs_shader_state.emitted_program)
>  		sctx->cs_shader_state.emitted_program = NULL;
>  
> -	si_shader_destroy(&program->shader);
> -	FREE(program);
> +	si_compute_reference(&program, NULL);
>  }
Here you call "si_compute_reference" with parameter src=NULL and de-
reference it in that function. 
Likewise you call FREE on "program" and also dereference it there. 
(see below). 

>  
>  static void si_set_compute_resources(struct pipe_context * ctx_,
> diff --git a/src/gallium/drivers/radeonsi/si_compute.h
> b/src/gallium/drivers/radeonsi/si_compute.h
> index 268817b23a6..c19b701fc71 100644
> --- a/src/gallium/drivers/radeonsi/si_compute.h
> +++ b/src/gallium/drivers/radeonsi/si_compute.h
> @@ -24,11 +24,14 @@
>  #ifndef SI_COMPUTE_H
>  #define SI_COMPUTE_H
>  
> +#include "util/u_inlines.h"
> +
>  #include "si_shader.h"
>  
>  #define MAX_GLOBAL_BUFFERS 22
>  
>  struct si_compute {
> +	struct pipe_reference reference;
>  	struct si_screen *screen;
>  	struct tgsi_token *tokens;
>  	struct util_queue_fence ready;
> @@ -53,4 +56,15 @@ struct si_compute {
>  	unsigned uses_bindless_images:1;
>  };
>  
> +void si_destroy_compute(struct si_compute *program);
> +
> +static inline void
> +si_compute_reference(struct si_compute **dst, struct si_compute
> *src)
> +{
> +	if (pipe_reference(&(*dst)->reference, &src->reference))
+	si_destroy_compute(*dst);

When called from si_delete_compute_state *dst has been FREEed and src
is NULL. 

> +
> +	*dst = src;
> +}
> +
>  #endif /* SI_COMPUTE_H */