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

[Jan Vesely]

On Tue, May 26, 2015 at 5:45 PM, Marek Olšák <maraeo at gmail.com> wrote:

> 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.
>

wouldn't a functioning llvm-r600 backend for OCL, and the fact that SI uses
llvm for graphics mean that GL-llvm-r600 is lower maintenance solution?
I'm not familiar with GL codepaths, is the GL(tgsi)-llvm conversion so
complex (and different from SI) that it's better to use SB?

jan


>
> Marek
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150605/e397c843/attachment.html>