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.ves...@rutgers.edu> > wrote: > > Signed-off-by: Jan Vesely <jan.ves...@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@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev