[Mesa-dev] [PATCH] panfrost: Use ralloc() to allocate instructions to avoid leaking those objs

Alyssa Rosenzweig alyssa.rosenzweig at collabora.com
Wed Aug 28 14:08:18 UTC 2019


Looks good, r-b.

Is it tested?

On Wed, Aug 28, 2019 at 09:32:04AM +0200, Boris Brezillon wrote:
> Instructions attached to blocks are never explicitly freed. Let's
> use ralloc() to attach those objects to the compiler context so that
> they are automatically freed when the ctx object is freed.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  src/panfrost/midgard/compiler.h                | 13 +++++++------
>  src/panfrost/midgard/midgard_compile.c         |  2 +-
>  src/panfrost/midgard/midgard_derivatives.c     |  2 +-
>  src/panfrost/midgard/midgard_opt_invert.c      |  2 +-
>  src/panfrost/midgard/midgard_opt_perspective.c |  2 +-
>  src/panfrost/midgard/midgard_ra.c              |  4 ++--
>  src/panfrost/midgard/midgard_schedule.c        |  6 +++---
>  src/panfrost/midgard/mir_promote_uniforms.c    |  2 +-
>  8 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
> index f9ba31b5959d..68716f92b0b9 100644
> --- a/src/panfrost/midgard/compiler.h
> +++ b/src/panfrost/midgard/compiler.h
> @@ -288,9 +288,9 @@ typedef struct compiler_context {
>  /* Append instruction to end of current block */
>  
>  static inline midgard_instruction *
> -mir_upload_ins(struct midgard_instruction ins)
> +mir_upload_ins(struct compiler_context *ctx, struct midgard_instruction ins)
>  {
> -        midgard_instruction *heap = malloc(sizeof(ins));
> +        midgard_instruction *heap = ralloc(ctx, struct midgard_instruction);
>          memcpy(heap, &ins, sizeof(ins));
>          return heap;
>  }
> @@ -298,15 +298,17 @@ mir_upload_ins(struct midgard_instruction ins)
>  static inline midgard_instruction *
>  emit_mir_instruction(struct compiler_context *ctx, struct midgard_instruction ins)
>  {
> -        midgard_instruction *u = mir_upload_ins(ins);
> +        midgard_instruction *u = mir_upload_ins(ctx, ins);
>          list_addtail(&u->link, &ctx->current_block->instructions);
>          return u;
>  }
>  
>  static inline struct midgard_instruction *
> -mir_insert_instruction_before(struct midgard_instruction *tag, struct midgard_instruction ins)
> +mir_insert_instruction_before(struct compiler_context *ctx,
> +                              struct midgard_instruction *tag,
> +                              struct midgard_instruction ins)
>  {
> -        struct midgard_instruction *u = mir_upload_ins(ins);
> +        struct midgard_instruction *u = mir_upload_ins(ctx, ins);
>          list_addtail(&u->link, &tag->link);
>          return u;
>  }
> @@ -315,7 +317,6 @@ static inline void
>  mir_remove_instruction(struct midgard_instruction *ins)
>  {
>          list_del(&ins->link);
> -        free(ins);
>  }
>  
>  static inline midgard_instruction*
> diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c
> index 74511b278d16..74796b661df6 100644
> --- a/src/panfrost/midgard/midgard_compile.c
> +++ b/src/panfrost/midgard/midgard_compile.c
> @@ -1962,7 +1962,7 @@ inline_alu_constants(compiler_context *ctx)
>                                  alu->src[1] = scratch;
>  
>                                  /* Inject us -before- the last instruction which set r31 */
> -                                mir_insert_instruction_before(mir_prev_op(alu), ins);
> +                                mir_insert_instruction_before(ctx, mir_prev_op(alu), ins);
>                          }
>                  }
>          }
> diff --git a/src/panfrost/midgard/midgard_derivatives.c b/src/panfrost/midgard/midgard_derivatives.c
> index ce45b46ecb9c..bfeae5077fce 100644
> --- a/src/panfrost/midgard/midgard_derivatives.c
> +++ b/src/panfrost/midgard/midgard_derivatives.c
> @@ -148,7 +148,7 @@ midgard_lower_derivatives(compiler_context *ctx, midgard_block *block)
>                  dup.texture.in_reg_swizzle = SWIZZLE_ZWWW;
>  
>                  /* Insert the new instruction */
> -                mir_insert_instruction_before(mir_next_op(ins), dup);
> +                mir_insert_instruction_before(ctx, mir_next_op(ins), dup);
>  
>                  /* TODO: Set .cont/.last automatically via dataflow analysis */
>                  ctx->texture_op_count++;
> diff --git a/src/panfrost/midgard/midgard_opt_invert.c b/src/panfrost/midgard/midgard_opt_invert.c
> index c3dc8142663e..729169f9a4a8 100644
> --- a/src/panfrost/midgard/midgard_opt_invert.c
> +++ b/src/panfrost/midgard/midgard_opt_invert.c
> @@ -56,7 +56,7 @@ midgard_lower_invert(compiler_context *ctx, midgard_block *block)
>  
>                  ins->dest = temp;
>                  ins->invert = false;
> -                mir_insert_instruction_before(mir_next_op(ins), not);
> +                mir_insert_instruction_before(ctx, mir_next_op(ins), not);
>          }
>  }
>  
> diff --git a/src/panfrost/midgard/midgard_opt_perspective.c b/src/panfrost/midgard/midgard_opt_perspective.c
> index feec5a5be390..aa4c58c470c2 100644
> --- a/src/panfrost/midgard/midgard_opt_perspective.c
> +++ b/src/panfrost/midgard/midgard_opt_perspective.c
> @@ -125,7 +125,7 @@ midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block)
>                          }
>                  };
>  
> -                mir_insert_instruction_before(ins, accel);
> +                mir_insert_instruction_before(ctx, ins, accel);
>                  mir_remove_instruction(ins);
>  
>                  progress |= true;
> diff --git a/src/panfrost/midgard/midgard_ra.c b/src/panfrost/midgard/midgard_ra.c
> index 16cb31f04135..9125d0f03925 100644
> --- a/src/panfrost/midgard/midgard_ra.c
> +++ b/src/panfrost/midgard/midgard_ra.c
> @@ -499,13 +499,13 @@ mir_lower_special_reads(compiler_context *ctx)
>                                  if (hazard_write) {
>                                          midgard_instruction *use = mir_next_op(pre_use);
>                                          assert(use);
> -                                        mir_insert_instruction_before(use, m);
> +                                        mir_insert_instruction_before(ctx, use, m);
>                                          mir_rewrite_index_dst_single(pre_use, i, idx);
>                                  } else {
>                                          idx = spill_idx++;
>                                          m = v_mov(i, blank_alu_src, idx);
>                                          m.mask = mir_mask_of_read_components(pre_use, i);
> -                                        mir_insert_instruction_before(pre_use, m);
> +                                        mir_insert_instruction_before(ctx, pre_use, m);
>                                          mir_rewrite_index_src_single(pre_use, i, idx);
>                                  }
>                          }
> diff --git a/src/panfrost/midgard/midgard_schedule.c b/src/panfrost/midgard/midgard_schedule.c
> index 60ad5ebb79c8..5c6878e9f6b0 100644
> --- a/src/panfrost/midgard/midgard_schedule.c
> +++ b/src/panfrost/midgard/midgard_schedule.c
> @@ -641,7 +641,7 @@ midgard_pair_load_store(compiler_context *ctx, midgard_block *block)
>  
>                                  /* We found one! Move it up to pair and remove it from the old location */
>  
> -                                mir_insert_instruction_before(ins, *c);
> +                                mir_insert_instruction_before(ctx, ins, *c);
>                                  mir_remove_instruction(c);
>  
>                                  break;
> @@ -805,7 +805,7 @@ static void mir_spill_register(
>                          /* Hint: don't rewrite this node */
>                          st.hint = true;
>  
> -                        mir_insert_instruction_before(mir_next_op(ins), st);
> +                        mir_insert_instruction_before(ctx, mir_next_op(ins), st);
>  
>                          if (!is_special)
>                                  ctx->spills++;
> @@ -873,7 +873,7 @@ static void mir_spill_register(
>  
>                                  st.mask = read_mask;
>  
> -                                mir_insert_instruction_before(before, st);
> +                                mir_insert_instruction_before(ctx, before, st);
>                                 // consecutive_skip = true;
>                          } else {
>                                  /* Special writes already have their move spilled in */
> diff --git a/src/panfrost/midgard/mir_promote_uniforms.c b/src/panfrost/midgard/mir_promote_uniforms.c
> index 500230f78205..428710e5154f 100644
> --- a/src/panfrost/midgard/mir_promote_uniforms.c
> +++ b/src/panfrost/midgard/mir_promote_uniforms.c
> @@ -103,7 +103,7 @@ midgard_promote_uniforms(compiler_context *ctx, unsigned promoted_count)
>                  if (needs_move) {
>                          midgard_instruction mov = v_mov(promoted, blank_alu_src, ins->dest);
>                          mov.mask = ins->mask;
> -                        mir_insert_instruction_before(ins, mov);
> +                        mir_insert_instruction_before(ctx, ins, mov);
>                  } else {
>                          mir_rewrite_index_src_swizzle(ctx, ins->dest,
>                                          promoted, swizzle_of(nr_components));
> -- 
> 2.21.0
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190828/71dc7112/attachment.sig>


More information about the mesa-dev mailing list