Re: [Mesa-dev] [PATCH 06/61] nir/validator: Validate that all used variables exist

[Jason Ekstrand]

On Thu, Mar 29, 2018 at 2:19 PM, Kenneth Graunke 
wrote:

> On Friday, March 23, 2018 2:42:12 PM PDT Jason Ekstrand wrote:
> > We were validating this for locals but nothing else.
> > ---
> >  src/compiler/nir/nir_validate.c | 16 +---
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_
> validate.c
> > index a49948f..e9d6bd5 100644
> > --- a/src/compiler/nir/nir_validate.c
> > +++ b/src/compiler/nir/nir_validate.c
> > @@ -96,7 +96,9 @@ typedef struct {
> > /* bitset of registers we have currently found; used to check
> uniqueness */
> > BITSET_WORD *regs_found;
> >
> > -   /* map of local variable -> function implementation where it is
> defined */
> > +   /* map of variable -> function implementation where it is defined or
> NULL
> > +* if it is a global variable
> > +*/
> > struct hash_table *var_defs;
> >
> > /* map of instruction/var/etc to failed assert string */
> > @@ -448,12 +450,10 @@ validate_deref_chain(nir_deref *deref,
> nir_variable_mode mode,
> >  static void
> >  validate_var_use(nir_variable *var, validate_state *state)
> >  {
> > -   if (var->data.mode == nir_var_local) {
> > -  struct hash_entry *entry = _mesa_hash_table_search(state->var_defs,
> var);
> > -
> > -  validate_assert(state, entry);
> > +   struct hash_entry *entry = _mesa_hash_table_search(state->var_defs,
> var);
> > +   validate_assert(state, entry);
> > +   if (var->data.mode == nir_var_local)
> >validate_assert(state, (nir_function_impl *) entry->data ==
> state->impl);
> > -   }
> >  }
> >
> >  static void
> > @@ -1000,7 +1000,9 @@ validate_var_decl(nir_variable *var, bool
> is_global, validate_state *state)
> >  * support)
> >  */
> >
> > -   if (!is_global) {
> > +   if (is_global) {
> > +  _mesa_hash_table_insert(state->var_defs, var, NULL);
> > +   } else {
> >_mesa_hash_table_insert(state->var_defs, var, state->impl);
> > }
>
> I'd personally do
>
>_mesa_hash_table_insert(state->var_defs, var,
>is_global ? NULL : state->impl);
>
> since we want to insert into the same set either way, just with NULL for
> the impl if there isn't one.  Doesn't matter though, your call.
>

Good call


> Patches 1-6 are:
> Reviewed-by: Kenneth Graunke 
>

Thanks!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/61] nir/validator: Validate that all used variables exist

[Kenneth Graunke]

On Friday, March 23, 2018 2:42:12 PM PDT Jason Ekstrand wrote:
> We were validating this for locals but nothing else.
> ---
>  src/compiler/nir/nir_validate.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
> index a49948f..e9d6bd5 100644
> --- a/src/compiler/nir/nir_validate.c
> +++ b/src/compiler/nir/nir_validate.c
> @@ -96,7 +96,9 @@ typedef struct {
> /* bitset of registers we have currently found; used to check uniqueness 
> */
> BITSET_WORD *regs_found;
>  
> -   /* map of local variable -> function implementation where it is defined */
> +   /* map of variable -> function implementation where it is defined or NULL
> +* if it is a global variable
> +*/
> struct hash_table *var_defs;
>  
> /* map of instruction/var/etc to failed assert string */
> @@ -448,12 +450,10 @@ validate_deref_chain(nir_deref *deref, 
> nir_variable_mode mode,
>  static void
>  validate_var_use(nir_variable *var, validate_state *state)
>  {
> -   if (var->data.mode == nir_var_local) {
> -  struct hash_entry *entry = _mesa_hash_table_search(state->var_defs, 
> var);
> -
> -  validate_assert(state, entry);
> +   struct hash_entry *entry = _mesa_hash_table_search(state->var_defs, var);
> +   validate_assert(state, entry);
> +   if (var->data.mode == nir_var_local)
>validate_assert(state, (nir_function_impl *) entry->data == 
> state->impl);
> -   }
>  }
>  
>  static void
> @@ -1000,7 +1000,9 @@ validate_var_decl(nir_variable *var, bool is_global, 
> validate_state *state)
>  * support)
>  */
>  
> -   if (!is_global) {
> +   if (is_global) {
> +  _mesa_hash_table_insert(state->var_defs, var, NULL);
> +   } else {
>_mesa_hash_table_insert(state->var_defs, var, state->impl);
> }

I'd personally do

   _mesa_hash_table_insert(state->var_defs, var,
   is_global ? NULL : state->impl);

since we want to insert into the same set either way, just with NULL for
the impl if there isn't one.  Doesn't matter though, your call.

Patches 1-6 are:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev