[Mesa-dev] [PATCH] compiler: nir: opt_large_constants: don't consider empty sized variables

[Jason Ekstrand]

On Thu, Aug 23, 2018 at 7:04 AM Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:

> We're hitting an assert in gfxbench because one of the local variable is a
> sampler :
>

Of course we were.... That's because gfxbench does illegal things like
creating local variables which are samplers. :-(  I don't think such a
variable is being used so maybe we can just ensure that dead_variables is
run on locals before opt_large_constants.  Or maybe we can improve the
spirv_to_nir hack to never add the variable to the IR in the first place.


> testfw_app: ../src/compiler/nir_types.cpp:551: void
> glsl_get_natural_size_align_bytes(const glsl_type*, unsigned int*, unsigned
> int*): Assertion `!"type does not have a natural size"' failed.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Fixes: 1235850522cd5e ("nir: Add a large constants optimization pass")
> ---
>  src/compiler/nir/nir_opt_large_constants.c | 10 +++++++++-
>  src/compiler/nir_types.cpp                 |  4 +++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_large_constants.c
> b/src/compiler/nir/nir_opt_large_constants.c
> index 25a921963df..5dfb07aed27 100644
> --- a/src/compiler/nir/nir_opt_large_constants.c
> +++ b/src/compiler/nir/nir_opt_large_constants.c
> @@ -178,11 +178,19 @@ nir_opt_large_constants(nir_shader *shader,
>              nir_variable *var = nir_deref_instr_get_variable(dst_deref);
>              assert(var->data.mode == nir_var_local);
>
> +            struct var_info *info = &var_infos[var->data.index];
> +            /* Variable with empty natural size (samplers, images,
> etc...) are
> +             * not considered constant.
> +             */
> +            unsigned var_size, var_align;
> +            size_align(var->type, &var_size, &var_align);
> +            if (!var_size)
> +               info->is_constant = false;
> +
>              /* We only consider variables constant if they only have
> constant
>               * stores, all the stores come before any reads, and all
> stores
>               * come in the first block.  We also can't handle indirect
> stores.
>               */
> -            struct var_info *info = &var_infos[var->data.index];
>              if (!src_is_const || info->found_read || !first_block ||
>                  nir_deref_instr_has_indirect(dst_deref))
>                 info->is_constant = false;
> diff --git a/src/compiler/nir_types.cpp b/src/compiler/nir_types.cpp
> index c8a29404969..e3a2fc2eac0 100644
> --- a/src/compiler/nir_types.cpp
> +++ b/src/compiler/nir_types.cpp
> @@ -548,7 +548,9 @@ glsl_get_natural_size_align_bytes(const struct
> glsl_type *type,
>     case GLSL_TYPE_ERROR:
>     case GLSL_TYPE_INTERFACE:
>     case GLSL_TYPE_FUNCTION:
> -      unreachable("type does not have a natural size");
> +      *align = 1;
> +      *size = 0;
> +      break;
>     }
>  }
>
> --
> 2.18.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180823/3dca534a/attachment.html>