<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 29, 2018 at 2:19 PM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Friday, March 23, 2018 2:42:12 PM PDT Jason Ekstrand wrote:<br>
> We were validating this for locals but nothing else.<br>
> ---<br>
>  src/compiler/nir/nir_validate.<wbr>c | 16 +++++++++-------<br>
>  1 file changed, 9 insertions(+), 7 deletions(-)<br>
><br>
> diff --git a/src/compiler/nir/nir_<wbr>validate.c b/src/compiler/nir/nir_<wbr>validate.c<br>
> index a49948f..e9d6bd5 100644<br>
> --- a/src/compiler/nir/nir_<wbr>validate.c<br>
> +++ b/src/compiler/nir/nir_<wbr>validate.c<br>
> @@ -96,7 +96,9 @@ typedef struct {<br>
>     /* bitset of registers we have currently found; used to check uniqueness */<br>
>     BITSET_WORD *regs_found;<br>
><br>
> -   /* map of local variable -> function implementation where it is defined */<br>
> +   /* map of variable -> function implementation where it is defined or NULL<br>
> +    * if it is a global variable<br>
> +    */<br>
>     struct hash_table *var_defs;<br>
><br>
>     /* map of instruction/var/etc to failed assert string */<br>
> @@ -448,12 +450,10 @@ validate_deref_chain(nir_deref *deref, nir_variable_mode mode,<br>
>  static void<br>
>  validate_var_use(nir_variable *var, validate_state *state)<br>
>  {<br>
> -   if (var->data.mode == nir_var_local) {<br>
> -      struct hash_entry *entry = _mesa_hash_table_search(state-<wbr>>var_defs, var);<br>
> -<br>
> -      validate_assert(state, entry);<br>
> +   struct hash_entry *entry = _mesa_hash_table_search(state-<wbr>>var_defs, var);<br>
> +   validate_assert(state, entry);<br>
> +   if (var->data.mode == nir_var_local)<br>
>        validate_assert(state, (nir_function_impl *) entry->data == state->impl);<br>
> -   }<br>
>  }<br>
><br>
>  static void<br>
> @@ -1000,7 +1000,9 @@ validate_var_decl(nir_variable *var, bool is_global, validate_state *state)<br>
>      * support)<br>
>      */<br>
><br>
> -   if (!is_global) {<br>
> +   if (is_global) {<br>
> +      _mesa_hash_table_insert(state-<wbr>>var_defs, var, NULL);<br>
> +   } else {<br>
>        _mesa_hash_table_insert(state-<wbr>>var_defs, var, state->impl);<br>
>     }<br>
<br>
</div></div>I'd personally do<br>
<br>
   _mesa_hash_table_insert(state-<wbr>>var_defs, var,<br>
                           is_global ? NULL : state->impl);<br>
<br>
since we want to insert into the same set either way, just with NULL for<br>
the impl if there isn't one.  Doesn't matter though, your call.<br></blockquote><div><br></div><div>Good call<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Patches 1-6 are:<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
</blockquote></div><br></div><div class="gmail_extra">Thanks!<br></div></div>