[Mesa-dev] [PATCH 3/3] r600, compute: Plug couple of memory leaks

Jan Vesely jan.vesely at rutgers.edu
Fri Jun 5 08:02:02 PDT 2015


sorry for the delay, I'm just getting settled.

On Tue, 2015-05-26 at 11:29 +0200, Marek Olšák wrote:
> The new functions in gallium/radeon shouldn't be inline, because
> inlining them isn't critical for performance.

should I avoid header definitions entirely, or are static (not inline)
functions OK?

> 
> Also, I don't see a point in functions that only contain one line.
> Especially radeon_llvm_dispose_kernel_module looks like an 
> unnecessary wrapper.

I generally use wrappers to achieve symmetric interface. i.e.
radeon_shader_binary_init
radeon_shader_binary_clean
look nicer then using memset in place of the first one.
Similarly
radeon_llvm_get_kernel_module
radeon_llvm_dispose_kernel_module

If that's not desirable, I'll remove the one-liners (binary_init,
dispose_kernel_modules).

Jan

> 
> Marek
> 
> On Tue, May 26, 2015 at 2:05 AM, Jan Vesely <jan.vesely at rutgers.edu> 
> wrote:
> > Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> > ---
> >  src/gallium/drivers/r600/evergreen_compute.c  | 24 
> > +++++++++++++++++++++---
> >  src/gallium/drivers/r600/r600_llvm.c          | 14 +++++++++-----
> >  src/gallium/drivers/r600/r600_llvm.h          |  2 ++
> >  src/gallium/drivers/radeon/r600_pipe_common.h | 17 
> > +++++++++++++++++
> >  src/gallium/drivers/radeon/radeon_llvm_util.h |  4 ++++
> >  5 files changed, 53 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/r600/evergreen_compute.c 
> > b/src/gallium/drivers/r600/evergreen_compute.c
> > index 25f5f7d..d2acd1a 100644
> > --- a/src/gallium/drivers/r600/evergreen_compute.c
> > +++ b/src/gallium/drivers/r600/evergreen_compute.c
> > @@ -225,7 +225,7 @@ void *evergreen_create_compute_state(
> >                 }
> >         }
> >  #else
> > -       memset(&shader->binary, 0, sizeof(shader->binary));
> > +       radeon_shader_binary_init(&shader->binary);
> >         radeon_elf_read(code, header->num_bytes, &shader->binary, 
> > true);
> >         r600_create_shader(&shader->bc, &shader->binary, 
> > &use_kill);
> > 
> > @@ -261,13 +261,31 @@ void *evergreen_create_compute_state(
> >         return shader;
> >  }
> > 
> > -void evergreen_delete_compute_state(struct pipe_context *ctx, 
> > void* state)
> > +void evergreen_delete_compute_state(struct pipe_context *ctx_, 
> > void* state)
> >  {
> > -       struct r600_pipe_compute *shader = (struct 
> > r600_pipe_compute *)state;
> > +       struct r600_context *ctx = (struct r600_context *)ctx_;
> > +       COMPUTE_DBG(ctx->screen, "*** 
> > evergreen_delete_compute_state\n");
> > +       struct r600_pipe_compute *shader = state;
> > 
> >         if (!shader)
> >                 return;
> > 
> > +#ifdef HAVE_OPENCL
> > +#if HAVE_LLVM < 0x0306
> > +       for (unsigned i = 0; i < shader->num_kernels; i++) {
> > +               struct r600_kernel *kernel = &shader->kernels[i];
> > +               radeon_llvm_dispose_kernel_module(kernel
> > ->llvm_module);
> > +       }
> > +       FREE(shader->kernels);
> > +       LLVMContextDispose(shader->llvm_ctx);
> > +#else
> > +       radeon_shader_binary_clean(&shader->binary);
> > +       r600_destroy_shader(&shader->bc);
> > +
> > +       /* TODO destroy shader->code_bo, shader->const_bo
> > +        * we'll need something like r600_buffer_free */
> > +#endif
> > +#endif
> >         FREE(shader);
> >  }
> > 
> > diff --git a/src/gallium/drivers/r600/r600_llvm.c 
> > b/src/gallium/drivers/r600/r600_llvm.c
> > index 94085fc..ac34d5c 100644
> > --- a/src/gallium/drivers/r600/r600_llvm.c
> > +++ b/src/gallium/drivers/r600/r600_llvm.c
> > @@ -872,6 +872,12 @@ unsigned r600_create_shader(struct 
> > r600_bytecode *bc,
> >         return 0;
> >  }
> > 
> > +void r600_destroy_shader(struct r600_bytecode *bc)
> > +{
> > +       FREE(bc->bytecode);
> > +       FREE(bc->rodata);
> > +}
> > +
> >  unsigned r600_llvm_compile(
> >         LLVMModuleRef mod,
> >         enum radeon_family family,
> > @@ -883,15 +889,13 @@ unsigned r600_llvm_compile(
> >         struct radeon_shader_binary binary;
> >         const char * gpu_family = 
> > r600_get_llvm_processor_name(family);
> > 
> > -       memset(&binary, 0, sizeof(struct radeon_shader_binary));
> > +       radeon_shader_binary_init(&binary);
> > +
> >         r = radeon_llvm_compile(mod, &binary, gpu_family, dump, 
> > NULL);
> > 
> >         r = r600_create_shader(bc, &binary, use_kill);
> > 
> > -       FREE(binary.code);
> > -       FREE(binary.config);
> > -       FREE(binary.rodata);
> > -       FREE(binary.global_symbol_offsets);
> > +       radeon_shader_binary_clean(&binary);
> > 
> >         return r;
> >  }
> > diff --git a/src/gallium/drivers/r600/r600_llvm.h 
> > b/src/gallium/drivers/r600/r600_llvm.h
> > index 9b5304d..442b312 100644
> > --- a/src/gallium/drivers/r600/r600_llvm.h
> > +++ b/src/gallium/drivers/r600/r600_llvm.h
> > @@ -28,6 +28,8 @@ unsigned r600_create_shader(struct r600_bytecode 
> > *bc,
> >                 const struct radeon_shader_binary *binary,
> >                 boolean *use_kill);
> > 
> > +void r600_destroy_shader(struct r600_bytecode *bc);
> > +
> >  void r600_shader_binary_read_config(const struct 
> > radeon_shader_binary *binary,
> >                 struct r600_bytecode *bc,
> >                 uint64_t symbol_offset,
> > diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h 
> > b/src/gallium/drivers/radeon/r600_pipe_common.h
> > index 6ce81d3..17383a3 100644
> > --- a/src/gallium/drivers/radeon/r600_pipe_common.h
> > +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
> > @@ -38,6 +38,7 @@
> > 
> >  #include "util/u_blitter.h"
> >  #include "util/list.h"
> > +#include "util/u_memory.h"
> >  #include "util/u_range.h"
> >  #include "util/u_slab.h"
> >  #include "util/u_suballoc.h"
> > @@ -132,6 +133,22 @@ struct radeon_shader_binary {
> >         int disassembled;
> >  };
> > 
> > +static inline void radeon_shader_binary_init(struct 
> > radeon_shader_binary *b)
> > +{
> > +       memset(b, 0, sizeof(*b));
> > +}
> > +
> > +static inline void radeon_shader_binary_clean(struct 
> > radeon_shader_binary *b)
> > +{
> > +       if (!b)
> > +               return;
> > +       FREE(b->code);
> > +       FREE(b->config);
> > +       FREE(b->rodata);
> > +       FREE(b->global_symbol_offsets);
> > +       FREE(b->relocs);
> > +}
> > +
> >  struct r600_resource {
> >         struct u_resource               b;
> > 
> > diff --git a/src/gallium/drivers/radeon/radeon_llvm_util.h 
> > b/src/gallium/drivers/radeon/radeon_llvm_util.h
> > index cc1932a..b0ff6d5 100644
> > --- a/src/gallium/drivers/radeon/radeon_llvm_util.h
> > +++ b/src/gallium/drivers/radeon/radeon_llvm_util.h
> > @@ -35,5 +35,9 @@ unsigned 
> > radeon_llvm_get_num_kernels(LLVMContextRef ctx,
> >                         const char *bitcode, unsigned bitcode_len);
> >  LLVMModuleRef radeon_llvm_get_kernel_module(LLVMContextRef ctx, 
> > unsigned index,
> >                         const char *bitcode, unsigned bitcode_len);
> > +static inline void radeon_llvm_dispose_kernel_module(LLVMModuleRef 
> > module)
> > +{
> > +       LLVMDisposeModule(module);
> > +}
> > 
> >  #endif
> > --
> > 2.1.0
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list