[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