[Mesa-dev] [PATCH] radeon/llvm: don't use a static array size for radeon_llvm_context::arrays

[Jan Vesely]

On Wed, 2015-05-27 at 00:36 +0200, Marek Olšák wrote:
> On Tue, May 26, 2015 at 10:19 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > On 26/05/15 14:04, Marek Olšák wrote:
> >> From: Marek Olšák <marek.olsak at amd.com>
> >>
> >> ---
> >>  src/gallium/drivers/radeon/radeon_llvm.h            |  5 ++---
> >>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 15 +++++++++++----
> >>  2 files changed, 13 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/src/gallium/drivers/radeon/radeon_llvm.h b/src/gallium/drivers/radeon/radeon_llvm.h
> >> index 8612ef8..ec4c343 100644
> >> --- a/src/gallium/drivers/radeon/radeon_llvm.h
> >> +++ b/src/gallium/drivers/radeon/radeon_llvm.h
> >> @@ -33,7 +33,6 @@
> >>
> >>  #define RADEON_LLVM_MAX_INPUTS 32 * 4
> >>  #define RADEON_LLVM_MAX_OUTPUTS 32 * 4
> >> -#define RADEON_LLVM_MAX_ARRAYS 16
> >>
> >>  #define RADEON_LLVM_INITIAL_CF_DEPTH 4
> >>
> >> @@ -130,8 +129,8 @@ struct radeon_llvm_context {
> >>       unsigned loop_depth;
> >>       unsigned loop_depth_max;
> >>
> >> -     struct tgsi_declaration_range arrays[RADEON_LLVM_MAX_ARRAYS];
> >> -     unsigned num_arrays;
> >> +     struct tgsi_declaration_range *arrays;
> >> +     unsigned max_num_arrays;
> >>
> >>       LLVMValueRef main_fn;
> >>
> >> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >> index 8638537..af5eb88 100644
> >> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >> @@ -85,8 +85,9 @@ get_array_range(struct lp_build_tgsi_context *bld_base,
> >>               unsigned File, const struct tgsi_ind_register *reg)
> >>  {
> >>       struct radeon_llvm_context * ctx = radeon_llvm_context(bld_base);
> >> +
> >>       if (File != TGSI_FILE_TEMPORARY || reg->ArrayID == 0 ||
> >> -            reg->ArrayID > RADEON_LLVM_MAX_ARRAYS) {
> >> +         reg->ArrayID-1 >= ctx->max_num_arrays) {
> >>               struct tgsi_declaration_range range;
> >>               range.First = 0;
> >>               range.Last = bld_base->info->file_max[File];
> >> @@ -252,8 +253,14 @@ static void emit_declaration(
> >>       }
> >>
> >>       case TGSI_FILE_TEMPORARY:
> >> -             if (decl->Declaration.Array && decl->Array.ArrayID <= RADEON_LLVM_MAX_ARRAYS)
> >> +             if (decl->Declaration.Array) {
> >> +                     if (decl->Array.ArrayID-1 >= ctx->max_num_arrays) {
> >> +                             ctx->max_num_arrays += 32;
> >> +                             ctx->arrays = realloc(ctx->arrays,
> > You should use the REALLOC wrapper, considering the FREE below.
> >
> >> +                                             sizeof(ctx->arrays[0]) * ctx->max_num_arrays);
> >> +                     }
> >>                       ctx->arrays[decl->Array.ArrayID - 1] = decl->Range;
> >> +             }
> >>               if (uses_temp_indirect_addressing(bld_base)) {
> >>                       lp_emit_declaration_soa(bld_base, decl);
> >>                       break;
> >> @@ -1432,8 +1439,6 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
> >>       /* Allocate outputs */
> >>       ctx->soa.outputs = ctx->outputs;
> >>
> >> -     ctx->num_arrays = 0;
> >> -
> >>       /* XXX: Is there a better way to initialize all this ? */
> >>
> >>       lp_set_default_actions(bld_base);
> >> @@ -1622,6 +1627,8 @@ void radeon_llvm_dispose(struct radeon_llvm_context * ctx)
> >>  {
> >>       LLVMDisposeModule(ctx->soa.bld_base.base.gallivm->module);
> >>       LLVMContextDispose(ctx->soa.bld_base.base.gallivm->context);
> >> +     FREE(ctx->arrays);
> >> +     ctx->arrays = NULL;
> > Not particularly familiar with the radeon driver(s) so this might come a
> > bit silly - shouldn't one zero max_num_arrays at this stage ?
> 
> Yes, I've sent a new patch for that.
> >
> > Similar counters (temps_count, output_reg_count) seems to be left
> > unchanged, while others (branch_depth_max, loop_depth_max) are reset.
> 
> I've sent a patch for temps_count. I'm not touching output_reg_count,
> because it's only used by the dying r600g LLVM backend.

just out of curiosity, why is it dying?

jan

> 
> Marek
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150526/af367aed/attachment.sig>