[Mesa-dev] [PATCH v2 1/4] nir: set default lod to texture opcodes that needed it but don't provide it

[Samuel Iglesias Gonsálvez]

On Wed, 2017-10-11 at 09:12 +0100, Lionel Landwerlin wrote:
> On 11/10/17 09:00, Samuel Iglesias Gonsálvez wrote:
> > On Tuesday, October 10, 2017 4:40:47 PM CEST Lionel Landwerlin
> > wrote:
> > > On 10/10/17 14:35, Samuel Iglesias Gonsálvez wrote:
> > > > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > > > ---
> > > > 
> > > >   src/compiler/nir/nir_lower_tex.c | 68
> > > >   ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68
> > > >   insertions(+)
> > > > 
> > > > diff --git a/src/compiler/nir/nir_lower_tex.c
> > > > b/src/compiler/nir/nir_lower_tex.c index
> > > > 65681decb1c..d3380710405 100644
> > > > --- a/src/compiler/nir/nir_lower_tex.c
> > > > +++ b/src/compiler/nir/nir_lower_tex.c
> > > > @@ -717,6 +717,52 @@ linearize_srgb_result(nir_builder *b,
> > > > nir_tex_instr
> > > > *tex)> 
> > > >                                     result->parent_instr);
> > > >   
> > > >   }
> > > > 
> > > > +static void
> > > > +set_default_lod(nir_builder *b, nir_tex_instr *tex)
> > > > +{
> > > > +   b->cursor = nir_before_instr(&tex->instr);
> > > > +
> > > > +   /* We are going to emit the same texture but adding a
> > > > default LOD.
> > > > +    */
> > > > +   int num_srcs = tex->num_srcs + 1;
> > > > +   nir_tex_instr *new = nir_tex_instr_create(b->shader,
> > > > num_srcs);
> > > > +
> > > > +   new->op = tex->op;
> > > > +   new->sampler_dim = tex->sampler_dim;
> > > > +   new->texture_index = tex->texture_index;
> > > > +   new->dest_type = tex->dest_type;
> > > > +   new->is_array = tex->is_array;
> > > > +   new->is_shadow = tex->is_shadow;
> > > > +   new->is_new_style_shadow = tex->is_new_style_shadow;
> > > > +   new->sampler_index = tex->sampler_index;
> > > > +   new->texture = nir_deref_var_clone(tex->texture, new);
> > > > +   new->sampler = nir_deref_var_clone(tex->sampler, new);
> > > > +   new->coord_components = tex->coord_components;
> > > 
> > > There are a couple of fields you're not copying : component &
> > > texture_array_size
> > > Not 100% sure whether they need to be.
> > > 
> > 
> > I added them locally.
> > 
> > > > +
> > > > +   nir_ssa_dest_init(&new->instr, &new->dest, 4, 32, NULL);
> > > > +
> > > > +   int src_num = 0;
> > > > +   for (int i = 0; i < tex->num_srcs; i++) {
> > > > +      nir_src_copy(&new->src[src_num].src, &tex->src[i].src,
> > > > new);
> > > > +      new->src[src_num].src_type = tex->src[i].src_type;
> > > > +      src_num++;
> > > > +   }
> > > > +
> > > > +   new->src[src_num].src = nir_src_for_ssa(nir_imm_int(b, 0));
> > > > +   new->src[src_num].src_type = nir_tex_src_lod;
> > > 
> > > I think you could get rid of the src_num variable and just use
> > > (new->num_srcs - 1) to set the default lod src.
> > > 
> > 
> > Done.
> > 
> > Does it get your R-b?
> > 
> > Thanks,
> > 
> > Sam
>  
> Thanks!
> Although I think Eric has a point avoid about memcpy(), since I used
> roughly the same code in the ycbcr anv pass, I'll try to come up with
> a helper.
> Patches 1-3 are :
> 
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> 

I forgot to reply before. I will wait for your helper then.
Please add me in Cc so I am aware when you submit it for review :)

Thanks,

Sam

> > > > +   src_num++;
> > > > +
> > > > +   assert(src_num == num_srcs);
> > > > +
> > > > +   nir_ssa_dest_init(&new->instr, &new->dest,
> > > > +                     tex->dest.ssa.num_components, 32, NULL);
> > > > +   nir_builder_instr_insert(b, &new->instr);
> > > > +
> > > > +   nir_ssa_def_rewrite_uses(&tex->dest.ssa,
> > > > nir_src_for_ssa(&new->dest.ssa)); +
> > > > +   nir_instr_remove(&tex->instr);
> > > > +}
> > > > +
> > > > 
> > > >   static bool
> > > >   nir_lower_tex_block(nir_block *block, nir_builder *b,
> > > >   
> > > >                       const nir_lower_tex_options *options)
> > > > 
> > > > @@ -813,6 +859,28 @@ nir_lower_tex_block(nir_block *block,
> > > > nir_builder *b,
> > > > 
> > > >            progress = true;
> > > >            continue;
> > > >         
> > > >         }
> > > > 
> > > > +
> > > > +      /* TXF, TXS and TXL require a LOD but not everything we
> > > > implement
> > > > using those +       * three opcodes provides one.  Provide a
> > > > default LOD
> > > > of 0. +       */
> > > > +      if (tex->op == nir_texop_txf || tex->op == nir_texop_txs
> > > > ||
> > > > +          tex->op == nir_texop_txl || tex->op ==
> > > > nir_texop_query_levels
> > > > ||
> > > > +          (tex->op == nir_texop_tex && b->shader->stage !=
> > > > MESA_SHADER_FRAGMENT)) { +         int i;
> > > > +         bool has_lod = false;
> > > > +         for (i = 0; i < tex->num_srcs; i++) {
> > > > +            if (tex->src[i].src_type == nir_tex_src_lod) {
> > > > +               has_lod = true;
> > > > +               break;
> > > > +            }
> > > > +         }
> > > > +
> > > > +         if (!has_lod) {
> > > > +            set_default_lod(b, tex);
> > > > +            progress = true;
> > > > +            continue;
> > > > +         }
> > > > +      }
> > > > 
> > > >      }
> > > >      
> > > >      return progress;
> > 
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171016/aff3b43a/attachment-0001.sig>