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

[Marek Olšák]

On Wed, May 27, 2015 at 12:39 AM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> 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?

The OpenGL shader support is dying because the LLVM backend is not
used by default, it lacks some features, and SB should perform
equally, if not better.

The LLVM backend is still important for OpenCL though.

Marek