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

[Jason Ekstrand]

On Wed, Oct 11, 2017 at 9:42 AM, Eric Anholt <eric at anholt.net> wrote:

> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
>
> > [ Unknown signature status ]
> > On Tuesday, October 10, 2017 11:53:27 AM CEST Eric Anholt wrote:
> >> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> >> > 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;
> >>
> >> Couldn't we just make a new srcs array of num_srcs+1, memcpy the old
> >> srcs/types over, add our new use of the immediate 0 lod to it by
> >> manipulating the immediate's uses list
> >
> > This is an interesting approach.  I have done the following:
> >
> >    b->cursor = nir_before_instr(&tex->instr);
> >
> >    nir_tex_src *srcs = ralloc_array(tex, nir_tex_src, tex->num_srcs + 1);
> >    memcpy(srcs, tex->src, sizeof(nir_tex_src) * tex->num_srcs);
> >
> >    srcs[tex->num_srcs + 1].src = nir_src_for_ssa(nir_imm_int(b, 0));
> >    srcs[tex->num_srcs + 1].src_type = nir_tex_src_lod;
> >    tex->num_srcs++;
> >    ralloc_free(tex->src);
> >    tex->src = srcs;
> >
> > However, it crashes when validating ssa def. I also tried calling
> > nir_ssa_def_rewrite_uses() but I am not sure how to do it for this case,
> > probably I am missing something.
> >
> > How can I manipulate the immediate's uses list for this case? Can you
> provide
> > an example?
>
> When you nir_instr_insert() in your original code, add_defs_uses() will
> call add_use_cb() on this src.  I was thinking that you could just
> open-code that bit (or maybe we want to make the body of that function
> public?)
>
> Oh, wait, reallocating the srcs array would break the uses entries of
> all the other srcs, so you'd need to remove uses from the old list and
> add all of the new list.  It's less clear to me now whether this is a
> good way to go.
>

I think just resizing the sources array is the right thing to do.  However,
Lionel is 100% correct that we really need a helper for this.  There are
several places which have to deal with it and it's always a pain.
anv_nir_apply_pipeline_layout has examples of both growing and shrinking.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171011/6f65c0c1/attachment.html>