Re: [Mesa-dev] [PATCH 1/3] nir: handle no variable in derefs in some places

2018-07-08 Thread Jason Ekstrand
On Tue, Jul 3, 2018 at 11:25 PM Dave Airlie  wrote:

> From: Dave Airlie 
>
> ---
>  src/compiler/nir/nir_gather_info.c   | 2 ++
>  src/compiler/nir/nir_lower_indirect_derefs.c | 4 
>  src/compiler/nir/nir_lower_vars_to_ssa.c | 4 
>  3 files changed, 10 insertions(+)
>
> diff --git a/src/compiler/nir/nir_gather_info.c
> b/src/compiler/nir/nir_gather_info.c
> index 2b431e343e9..4bbdd967c2b 100644
> --- a/src/compiler/nir/nir_gather_info.c
> +++ b/src/compiler/nir/nir_gather_info.c
> @@ -233,6 +233,8 @@ gather_intrinsic_info(nir_intrinsic_instr *instr,
> nir_shader *shader,
>nir_deref_instr *deref = nir_src_as_deref(instr->src[0]);
>nir_variable *var = nir_deref_instr_get_variable(deref);
>
> +  if (!var)
> + break;
>

You should be able to change the if below to

if (deref->mode == nir_var_shader_in || deref->mode == nir_var_shader_out)

and then pull the nir_deref_instr_get_variable inside the if.


>if (var->data.mode == nir_var_shader_in ||
>var->data.mode == nir_var_shader_out) {
>   bool is_output_read = false;
> diff --git a/src/compiler/nir/nir_lower_indirect_derefs.c
> b/src/compiler/nir/nir_lower_indirect_derefs.c
> index d85c1704222..be39e1098ed 100644
> --- a/src/compiler/nir/nir_lower_indirect_derefs.c
> +++ b/src/compiler/nir/nir_lower_indirect_derefs.c
> @@ -131,6 +131,8 @@ lower_indirect_derefs_block(nir_block *block,
> nir_builder *b,
>   continue;
>
>nir_deref_instr *deref = nir_src_as_deref(intrin->src[0]);
> +  if (!deref)
> + continue;
>

Is this a real problem?  intrin->src[0] should always point at a deref even
if that deref is just a cast.


>/* Walk the deref chain back to the base and look for indirects */
>bool has_indirect = false;
> @@ -141,6 +143,8 @@ lower_indirect_derefs_block(nir_block *block,
> nir_builder *b,
>  has_indirect = true;
>
>   base = nir_deref_instr_parent(base);
> + if (!base)
> +break;
>

This isn't sufficient.  You need to make some case below continue if base
== NULL.


>}
>
>if (!has_indirect)
> diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c
> b/src/compiler/nir/nir_lower_vars_to_ssa.c
> index 3f37acaed33..dcef9b8e221 100644
> --- a/src/compiler/nir/nir_lower_vars_to_ssa.c
> +++ b/src/compiler/nir/nir_lower_vars_to_ssa.c
> @@ -142,6 +142,8 @@ static struct deref_node *
>  get_deref_node_recur(nir_deref_instr *deref,
>   struct lower_variables_state *state)
>  {
> +   if (!deref)
> +  return NULL;
> if (deref->deref_type == nir_deref_type_var)
>return get_deref_node_for_var(deref->var, state);
>
> @@ -198,6 +200,8 @@ get_deref_node_recur(nir_deref_instr *deref,
>
>return parent->wildcard;
>
> +   case nir_deref_type_cast:
> +  return NULL;
>

I think the better thing to do here would be to just look at the deref mode
and bail if it's not a nir_var_local.  I just sent an untested patch that
does just that.


> default:
>unreachable("Invalid deref type");
> }
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/3] nir: handle no variable in derefs in some places

2018-07-04 Thread Dave Airlie
From: Dave Airlie 

---
 src/compiler/nir/nir_gather_info.c   | 2 ++
 src/compiler/nir/nir_lower_indirect_derefs.c | 4 
 src/compiler/nir/nir_lower_vars_to_ssa.c | 4 
 3 files changed, 10 insertions(+)

diff --git a/src/compiler/nir/nir_gather_info.c 
b/src/compiler/nir/nir_gather_info.c
index 2b431e343e9..4bbdd967c2b 100644
--- a/src/compiler/nir/nir_gather_info.c
+++ b/src/compiler/nir/nir_gather_info.c
@@ -233,6 +233,8 @@ gather_intrinsic_info(nir_intrinsic_instr *instr, 
nir_shader *shader,
   nir_deref_instr *deref = nir_src_as_deref(instr->src[0]);
   nir_variable *var = nir_deref_instr_get_variable(deref);
 
+  if (!var)
+ break;
   if (var->data.mode == nir_var_shader_in ||
   var->data.mode == nir_var_shader_out) {
  bool is_output_read = false;
diff --git a/src/compiler/nir/nir_lower_indirect_derefs.c 
b/src/compiler/nir/nir_lower_indirect_derefs.c
index d85c1704222..be39e1098ed 100644
--- a/src/compiler/nir/nir_lower_indirect_derefs.c
+++ b/src/compiler/nir/nir_lower_indirect_derefs.c
@@ -131,6 +131,8 @@ lower_indirect_derefs_block(nir_block *block, nir_builder 
*b,
  continue;
 
   nir_deref_instr *deref = nir_src_as_deref(intrin->src[0]);
+  if (!deref)
+ continue;
 
   /* Walk the deref chain back to the base and look for indirects */
   bool has_indirect = false;
@@ -141,6 +143,8 @@ lower_indirect_derefs_block(nir_block *block, nir_builder 
*b,
 has_indirect = true;
 
  base = nir_deref_instr_parent(base);
+ if (!base)
+break;
   }
 
   if (!has_indirect)
diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c 
b/src/compiler/nir/nir_lower_vars_to_ssa.c
index 3f37acaed33..dcef9b8e221 100644
--- a/src/compiler/nir/nir_lower_vars_to_ssa.c
+++ b/src/compiler/nir/nir_lower_vars_to_ssa.c
@@ -142,6 +142,8 @@ static struct deref_node *
 get_deref_node_recur(nir_deref_instr *deref,
  struct lower_variables_state *state)
 {
+   if (!deref)
+  return NULL;
if (deref->deref_type == nir_deref_type_var)
   return get_deref_node_for_var(deref->var, state);
 
@@ -198,6 +200,8 @@ get_deref_node_recur(nir_deref_instr *deref,
 
   return parent->wildcard;
 
+   case nir_deref_type_cast:
+  return NULL;
default:
   unreachable("Invalid deref type");
}
-- 
2.17.1

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