Re: [Mesa-dev] [PATCH 07/16] nir/lower_tex: Add support for lowering coordinate offsets

[Jason Ekstrand]

On Fri, Jul 22, 2016 at 1:11 AM, Pohjolainen, Topi <
topi.pohjolai...@intel.com> wrote:

> On Thu, Jul 21, 2016 at 09:21:53PM -0700, Jason Ekstrand wrote:
> > On i965, we can't support coordinate offsets for texelFetch or rectangle
> > textures.  Previously, we were doing this with a GLSL pass but we need to
> > do it in NIR if we want those workarounds for SPIR-V.
> >
> > Signed-off-by: Jason Ekstrand 
> > Cc: "12.0" 
> > ---
> >  src/compiler/nir/nir.h   | 10 
> >  src/compiler/nir/nir_lower_tex.c | 54
> 
> >  2 files changed, 64 insertions(+)
> >
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index d0f52b0..45f758c 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -2405,6 +2405,16 @@ typedef struct nir_lower_tex_options {
> > unsigned lower_txp;
> >
> > /**
> > +* If true, lower away nir_tex_src_offset for all texelfetch
> instructions.
> > +*/
> > +   bool lower_txf_offset;
> > +
> > +   /**
> > +* If true, lower away nir_tex_src_offset for all rect textures.
> > +*/
> > +   bool lower_rect_offset;
> > +
> > +   /**
> >  * If true, lower rect textures to 2D, using txs to fetch the
> >  * texture dimensions and dividing the texture coords by the
> >  * texture dims to normalize.
> > diff --git a/src/compiler/nir/nir_lower_tex.c
> b/src/compiler/nir/nir_lower_tex.c
> > index 0cf1071..a1280e1 100644
> > --- a/src/compiler/nir/nir_lower_tex.c
> > +++ b/src/compiler/nir/nir_lower_tex.c
> > @@ -128,6 +128,54 @@ project_src(nir_builder *b, nir_tex_instr *tex)
> > tex_instr_remove_src(tex, proj_index);
> >  }
> >
> > +static bool
> > +lower_offset(nir_builder *b, nir_tex_instr *tex)
> > +{
> > +   int offset_index = tex_instr_find_src(tex, nir_tex_src_offset);
>
> Could be 'const'.
>
> > +   if (offset_index < 0)
> > +  return false;
> > +
> > +   int coord_index = tex_instr_find_src(tex, nir_tex_src_coord);
>
> Same here.
>
> > +   assert(coord_index >= 0);
> > +
> > +   assert(tex->src[offset_index].src.is_ssa);
> > +   assert(tex->src[coord_index].src.is_ssa);
> > +   nir_ssa_def *offset = tex->src[offset_index].src.ssa;
> > +   nir_ssa_def *coord = tex->src[coord_index].src.ssa;
>
> In principle, it looks these could be declared constants as well:
>
>   const nir_ssa_def * const offset = tex->src[offset_index].src.ssa;
>   const nir_ssa_def * const coord = tex->src[coord_index].src.ssa;
>
> But further digging tells me that they can be only:
>
>   nir_ssa_def * const offset = tex->src[offset_index].src.ssa;
>   nir_ssa_def * const coord = tex->src[coord_index].src.ssa;
>
>
> Quite a few of the helpers in nir declare their inputs as read-write even
> though they only read the contents...
>

I have tried on multiple occasions to make NIR more const-safe.
Unfortunately, every attempt only seems to lead to frustration.  Some of
this is due to limitations in C (which are fixed in C11 which we can't use)
that make it very hard to make constructs that can handle either const or
non-const.  This, in turn, makes it very hard to constify things.  There is
also the fact that we use a lot of embedded linked lists which make a bunch
of things that look like they should be const non-const.  I've given up.
It's better to not claim any sort of constness than to try and claim it and
have it all be lies.


>
> > +
> > +   b->cursor = nir_before_instr(>instr);
> > +
> > +   nir_ssa_def *offset_coord;
> > +   if (nir_tex_instr_src_type(tex, coord_index) == nir_type_float) {
> > +  assert(tex->sampler_dim == GLSL_SAMPLER_DIM_RECT);
> > +  offset_coord = nir_fadd(b, coord, nir_i2f(b, offset));
> > +   } else {
> > +  offset_coord = nir_iadd(b, coord, offset);
> > +   }
> > +
> > +   if (tex->is_array) {
> > +  /* The offset is not applied to the array index */
> > +  if (tex->coord_components == 2) {
> > + offset_coord = nir_vec2(b, nir_channel(b, offset_coord, 0),
> > +nir_channel(b, coord, 1));
> > +  } else if (tex->coord_components == 3) {
> > + offset_coord = nir_vec3(b, nir_channel(b, offset_coord, 0),
> > +nir_channel(b, offset_coord, 1),
> > +nir_channel(b, coord, 2));
> > +  } else {
> > + unreachable("Invalid number of components");
> > +  }
> > +   }
> > +
> > +   nir_instr_rewrite_src(>instr, >src[coord_index].src,
> > + nir_src_for_ssa(offset_coord));
> > +
> > +   tex_instr_remove_src(tex, offset_index);
> > +
> > +   return true;
> > +}
> > +
> > +
> >  static nir_ssa_def *
> >  get_texture_size(nir_builder *b, nir_tex_instr *tex)
> >  {
> > @@ -458,6 +506,12 @@ nir_lower_tex_block(nir_block *block, nir_builder
> *b,
> >   progress = true;
> >}
> >
> > +  if ((tex->op == nir_texop_txf && 

Re: [Mesa-dev] [PATCH 07/16] nir/lower_tex: Add support for lowering coordinate offsets

[Pohjolainen, Topi]

On Thu, Jul 21, 2016 at 09:21:53PM -0700, Jason Ekstrand wrote:
> On i965, we can't support coordinate offsets for texelFetch or rectangle
> textures.  Previously, we were doing this with a GLSL pass but we need to
> do it in NIR if we want those workarounds for SPIR-V.
> 
> Signed-off-by: Jason Ekstrand 
> Cc: "12.0" 
> ---
>  src/compiler/nir/nir.h   | 10 
>  src/compiler/nir/nir_lower_tex.c | 54 
> 
>  2 files changed, 64 insertions(+)
> 
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index d0f52b0..45f758c 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2405,6 +2405,16 @@ typedef struct nir_lower_tex_options {
> unsigned lower_txp;
>  
> /**
> +* If true, lower away nir_tex_src_offset for all texelfetch instructions.
> +*/
> +   bool lower_txf_offset;
> +
> +   /**
> +* If true, lower away nir_tex_src_offset for all rect textures.
> +*/
> +   bool lower_rect_offset;
> +
> +   /**
>  * If true, lower rect textures to 2D, using txs to fetch the
>  * texture dimensions and dividing the texture coords by the
>  * texture dims to normalize.
> diff --git a/src/compiler/nir/nir_lower_tex.c 
> b/src/compiler/nir/nir_lower_tex.c
> index 0cf1071..a1280e1 100644
> --- a/src/compiler/nir/nir_lower_tex.c
> +++ b/src/compiler/nir/nir_lower_tex.c
> @@ -128,6 +128,54 @@ project_src(nir_builder *b, nir_tex_instr *tex)
> tex_instr_remove_src(tex, proj_index);
>  }
>  
> +static bool
> +lower_offset(nir_builder *b, nir_tex_instr *tex)
> +{
> +   int offset_index = tex_instr_find_src(tex, nir_tex_src_offset);

Could be 'const'.

> +   if (offset_index < 0)
> +  return false;
> +
> +   int coord_index = tex_instr_find_src(tex, nir_tex_src_coord);

Same here.

> +   assert(coord_index >= 0);
> +
> +   assert(tex->src[offset_index].src.is_ssa);
> +   assert(tex->src[coord_index].src.is_ssa);
> +   nir_ssa_def *offset = tex->src[offset_index].src.ssa;
> +   nir_ssa_def *coord = tex->src[coord_index].src.ssa;

In principle, it looks these could be declared constants as well:

  const nir_ssa_def * const offset = tex->src[offset_index].src.ssa;
  const nir_ssa_def * const coord = tex->src[coord_index].src.ssa;

But further digging tells me that they can be only:

  nir_ssa_def * const offset = tex->src[offset_index].src.ssa;
  nir_ssa_def * const coord = tex->src[coord_index].src.ssa;


Quite a few of the helpers in nir declare their inputs as read-write even
though they only read the contents...

> +
> +   b->cursor = nir_before_instr(>instr);
> +
> +   nir_ssa_def *offset_coord;
> +   if (nir_tex_instr_src_type(tex, coord_index) == nir_type_float) {
> +  assert(tex->sampler_dim == GLSL_SAMPLER_DIM_RECT);
> +  offset_coord = nir_fadd(b, coord, nir_i2f(b, offset));
> +   } else {
> +  offset_coord = nir_iadd(b, coord, offset);
> +   }
> +
> +   if (tex->is_array) {
> +  /* The offset is not applied to the array index */
> +  if (tex->coord_components == 2) {
> + offset_coord = nir_vec2(b, nir_channel(b, offset_coord, 0),
> +nir_channel(b, coord, 1));
> +  } else if (tex->coord_components == 3) {
> + offset_coord = nir_vec3(b, nir_channel(b, offset_coord, 0),
> +nir_channel(b, offset_coord, 1),
> +nir_channel(b, coord, 2));
> +  } else {
> + unreachable("Invalid number of components");
> +  }
> +   }
> +
> +   nir_instr_rewrite_src(>instr, >src[coord_index].src,
> + nir_src_for_ssa(offset_coord));
> +
> +   tex_instr_remove_src(tex, offset_index);
> +
> +   return true;
> +}
> +
> +
>  static nir_ssa_def *
>  get_texture_size(nir_builder *b, nir_tex_instr *tex)
>  {
> @@ -458,6 +506,12 @@ nir_lower_tex_block(nir_block *block, nir_builder *b,
>   progress = true;
>}
>  
> +  if ((tex->op == nir_texop_txf && options->lower_txf_offset) ||
> +  (tex->sampler_dim == GLSL_SAMPLER_DIM_RECT &&
> +   options->lower_rect_offset)) {
> + progress = lower_offset(b, tex) || progress;
> +  }
> +
>if ((tex->sampler_dim == GLSL_SAMPLER_DIM_RECT) && 
> options->lower_rect) {
>   lower_rect(b, tex);
>   progress = true;
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 07/16] nir/lower_tex: Add support for lowering coordinate offsets

[Jason Ekstrand]

On i965, we can't support coordinate offsets for texelFetch or rectangle
textures.  Previously, we were doing this with a GLSL pass but we need to
do it in NIR if we want those workarounds for SPIR-V.

Signed-off-by: Jason Ekstrand 
Cc: "12.0" 
---
 src/compiler/nir/nir.h   | 10 
 src/compiler/nir/nir_lower_tex.c | 54 
 2 files changed, 64 insertions(+)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index d0f52b0..45f758c 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2405,6 +2405,16 @@ typedef struct nir_lower_tex_options {
unsigned lower_txp;
 
/**
+* If true, lower away nir_tex_src_offset for all texelfetch instructions.
+*/
+   bool lower_txf_offset;
+
+   /**
+* If true, lower away nir_tex_src_offset for all rect textures.
+*/
+   bool lower_rect_offset;
+
+   /**
 * If true, lower rect textures to 2D, using txs to fetch the
 * texture dimensions and dividing the texture coords by the
 * texture dims to normalize.
diff --git a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
index 0cf1071..a1280e1 100644
--- a/src/compiler/nir/nir_lower_tex.c
+++ b/src/compiler/nir/nir_lower_tex.c
@@ -128,6 +128,54 @@ project_src(nir_builder *b, nir_tex_instr *tex)
tex_instr_remove_src(tex, proj_index);
 }
 
+static bool
+lower_offset(nir_builder *b, nir_tex_instr *tex)
+{
+   int offset_index = tex_instr_find_src(tex, nir_tex_src_offset);
+   if (offset_index < 0)
+  return false;
+
+   int coord_index = tex_instr_find_src(tex, nir_tex_src_coord);
+   assert(coord_index >= 0);
+
+   assert(tex->src[offset_index].src.is_ssa);
+   assert(tex->src[coord_index].src.is_ssa);
+   nir_ssa_def *offset = tex->src[offset_index].src.ssa;
+   nir_ssa_def *coord = tex->src[coord_index].src.ssa;
+
+   b->cursor = nir_before_instr(>instr);
+
+   nir_ssa_def *offset_coord;
+   if (nir_tex_instr_src_type(tex, coord_index) == nir_type_float) {
+  assert(tex->sampler_dim == GLSL_SAMPLER_DIM_RECT);
+  offset_coord = nir_fadd(b, coord, nir_i2f(b, offset));
+   } else {
+  offset_coord = nir_iadd(b, coord, offset);
+   }
+
+   if (tex->is_array) {
+  /* The offset is not applied to the array index */
+  if (tex->coord_components == 2) {
+ offset_coord = nir_vec2(b, nir_channel(b, offset_coord, 0),
+nir_channel(b, coord, 1));
+  } else if (tex->coord_components == 3) {
+ offset_coord = nir_vec3(b, nir_channel(b, offset_coord, 0),
+nir_channel(b, offset_coord, 1),
+nir_channel(b, coord, 2));
+  } else {
+ unreachable("Invalid number of components");
+  }
+   }
+
+   nir_instr_rewrite_src(>instr, >src[coord_index].src,
+ nir_src_for_ssa(offset_coord));
+
+   tex_instr_remove_src(tex, offset_index);
+
+   return true;
+}
+
+
 static nir_ssa_def *
 get_texture_size(nir_builder *b, nir_tex_instr *tex)
 {
@@ -458,6 +506,12 @@ nir_lower_tex_block(nir_block *block, nir_builder *b,
  progress = true;
   }
 
+  if ((tex->op == nir_texop_txf && options->lower_txf_offset) ||
+  (tex->sampler_dim == GLSL_SAMPLER_DIM_RECT &&
+   options->lower_rect_offset)) {
+ progress = lower_offset(b, tex) || progress;
+  }
+
   if ((tex->sampler_dim == GLSL_SAMPLER_DIM_RECT) && options->lower_rect) {
  lower_rect(b, tex);
  progress = true;
-- 
2.5.0.400.gff86faf

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