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

[Lionel Landwerlin]

On 23/08/2018 13:50, Jason Ekstrand wrote:
> On Thu, Aug 23, 2018 at 7:04 AM Lionel Landwerlin 
> <lionel.g.landwerlin at intel.com <mailto: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.


Indeed, it's not used.

Let me send a more sensible workaround/fix.


>     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
>     <mailto: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/c317a257/attachment.html>