[Mesa-dev] [PATCH 1/2] compiler/nir: add a is_image_sample_dref flag to texture instructions

Iago Toral itoral at igalia.com
Mon Apr 2 10:00:00 UTC 2018


On Wed, 2018-03-28 at 17:19 -0700, Jason Ekstrand wrote:
> How is this different from is_shadow?

>From the SPIR-V spec, OpTypeImage:
    
"Depth is whether or not this image is a depth image. (Note that
 whether or not depth comparisons are actually done is a property of
 the sampling opcode, not of this type declaration.)"

We only set is_shadow to true when the depth property is not zero, and
these tests are exercising the case where it is zero (but depth
comparison is still expected to happen because of the sampling opcode
used).

We can change this and set is_shadow to true unconditionally also when
any of these opcodes are involved, that also fixes the tests and is
simpler.

I didn't do this initially because I figured drivers could get confused
when in this scenario they see that the texture operation has is_shadow
set to true but the sampler type is not a shadow type (because the
image type has its depth property set to 0) , but maybe I was
overthiking things... looking at uses of is_shadow in the repository,
it seems that drivers only check is_shadow when they want to do depth
comparison and they don't look at anythig else, so it is probably just
fine to do this in general and it saves us from the hassle of adding a
new flag. I guess we can always add the new flag if someone hits a case
that needs to differentiate both scenarios anyway.

I'll send a new patch for review soon if Jenkins doesn't find any
issues.

Iago

> On March 28, 2018 02:33:50 Iago Toral Quiroga <itoral at igalia.com>
> wrote:
> 
> > So we can recognize image sampling instructions that involve a
> > depth
> > comparison against a reference, such as SPIR-V's
> > OpImageSample{Proj}Dref{Explicit,Implicit}Lod and we can
> > acknowledge
> > that they return a single scalar value instead of a vec4.
> > ---
> > src/compiler/nir/nir.h           | 9 +++++++++
> > src/compiler/nir/nir_clone.c     | 1 +
> > src/compiler/nir/nir_instr_set.c | 2 ++
> > src/compiler/nir/nir_lower_tex.c | 5 ++++-
> > src/compiler/nir/nir_serialize.c | 5 ++++-
> > 5 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index 0d207d0ea5..625092cd2b 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -1231,6 +1231,12 @@ typedef struct {
> >     */
> >    bool is_new_style_shadow;
> > 
> > +   /**
> > +    * If is_image_sample_dref is true, this is an image sample
> > with depth
> > +    * comparing.
> > +    */
> > +   bool is_image_sample_dref;
> > +
> >    /* gather component selector */
> >    unsigned component : 2;
> > 
> > @@ -1316,6 +1322,9 @@ nir_tex_instr_dest_size(const nir_tex_instr
> > *instr)
> >       if (instr->is_shadow && instr->is_new_style_shadow)
> >          return 1;
> > 
> > +      if (instr->is_image_sample_dref)
> > +         return 1;
> > +
> >       return 4;
> >    }
> > }
> > diff --git a/src/compiler/nir/nir_clone.c
> > b/src/compiler/nir/nir_clone.c
> > index bcfdaa7594..7d6cfd896f 100644
> > --- a/src/compiler/nir/nir_clone.c
> > +++ b/src/compiler/nir/nir_clone.c
> > @@ -415,6 +415,7 @@ clone_tex(clone_state *state, const
> > nir_tex_instr *tex)
> >    ntex->is_array = tex->is_array;
> >    ntex->is_shadow = tex->is_shadow;
> >    ntex->is_new_style_shadow = tex->is_new_style_shadow;
> > +   ntex->is_image_sample_dref = tex->is_image_sample_dref;
> >    ntex->component = tex->component;
> > 
> >    ntex->texture_index = tex->texture_index;
> > diff --git a/src/compiler/nir/nir_instr_set.c 
> > b/src/compiler/nir/nir_instr_set.c
> > index 9cb9ed43e8..5563f6f095 100644
> > --- a/src/compiler/nir/nir_instr_set.c
> > +++ b/src/compiler/nir/nir_instr_set.c
> > @@ -155,6 +155,7 @@ hash_tex(uint32_t hash, const nir_tex_instr
> > *instr)
> >    hash = HASH(hash, instr->is_array);
> >    hash = HASH(hash, instr->is_shadow);
> >    hash = HASH(hash, instr->is_new_style_shadow);
> > +   hash = HASH(hash, instr->is_image_sample_dref);
> >    unsigned component = instr->component;
> >    hash = HASH(hash, component);
> >    hash = HASH(hash, instr->texture_index);
> > @@ -310,6 +311,7 @@ nir_instrs_equal(const nir_instr *instr1,
> > const 
> > nir_instr *instr2)
> >           tex1->is_array != tex2->is_array ||
> >           tex1->is_shadow != tex2->is_shadow ||
> >           tex1->is_new_style_shadow != tex2->is_new_style_shadow ||
> > +          tex1->is_image_sample_dref != tex2->is_image_sample_dref 
> > ||
> >           tex1->component != tex2->component ||
> >          tex1->texture_index != tex2->texture_index ||
> >          tex1->texture_array_size != tex2->texture_array_size ||
> > diff --git a/src/compiler/nir/nir_lower_tex.c 
> > b/src/compiler/nir/nir_lower_tex.c
> > index 1062afd97f..03e7555679 100644
> > --- a/src/compiler/nir/nir_lower_tex.c
> > +++ b/src/compiler/nir/nir_lower_tex.c
> > @@ -114,6 +114,7 @@ get_texture_size(nir_builder *b, nir_tex_instr
> > *tex)
> >    txs->is_array = tex->is_array;
> >    txs->is_shadow = tex->is_shadow;
> >    txs->is_new_style_shadow = tex->is_new_style_shadow;
> > +   txs->is_image_sample_dref = tex->is_image_sample_dref;
> >    txs->texture_index = tex->texture_index;
> >    txs->texture = nir_deref_var_clone(tex->texture, txs);
> >    txs->sampler_index = tex->sampler_index;
> > @@ -343,6 +344,7 @@ replace_gradient_with_lod(nir_builder *b,
> > nir_ssa_def 
> > *lod, nir_tex_instr *tex)
> >    txl->is_array = tex->is_array;
> >    txl->is_shadow = tex->is_shadow;
> >    txl->is_new_style_shadow = tex->is_new_style_shadow;
> > +   txl->is_image_sample_dref = tex->is_image_sample_dref;
> >    txl->sampler_index = tex->sampler_index;
> >    txl->texture = nir_deref_var_clone(tex->texture, txl);
> >    txl->sampler = nir_deref_var_clone(tex->sampler, txl);
> > @@ -794,7 +796,8 @@ nir_lower_tex_block(nir_block *block,
> > nir_builder *b,
> > 
> >       if (((1 << tex->texture_index) & options->swizzle_result) &&
> >           !nir_tex_instr_is_query(tex) &&
> > -          !(tex->is_shadow && tex->is_new_style_shadow)) {
> > +          !(tex->is_shadow && tex->is_new_style_shadow) &&
> > +          !tex->is_image_sample_dref) {
> >          swizzle_result(b, tex, options->swizzles[tex-
> > >texture_index]);
> >          progress = true;
> >       }
> > diff --git a/src/compiler/nir/nir_serialize.c 
> > b/src/compiler/nir/nir_serialize.c
> > index 00df49c2ef..dcbe1f0c13 100644
> > --- a/src/compiler/nir/nir_serialize.c
> > +++ b/src/compiler/nir/nir_serialize.c
> > @@ -583,10 +583,11 @@ union packed_tex_data {
> >       unsigned is_array:1;
> >       unsigned is_shadow:1;
> >       unsigned is_new_style_shadow:1;
> > +      unsigned is_image_sample_dref:1;
> >       unsigned component:2;
> >       unsigned has_texture_deref:1;
> >       unsigned has_sampler_deref:1;
> > -      unsigned unused:10; /* Mark unused for valgrind. */
> > +      unsigned unused:9; /* Mark unused for valgrind. */
> >    } u;
> > };
> > 
> > @@ -607,6 +608,7 @@ write_tex(write_ctx *ctx, const nir_tex_instr
> > *tex)
> >       .u.is_array = tex->is_array,
> >       .u.is_shadow = tex->is_shadow,
> >       .u.is_new_style_shadow = tex->is_new_style_shadow,
> > +      .u.is_image_sample_dref = tex->is_image_sample_dref,
> >       .u.component = tex->component,
> >       .u.has_texture_deref = tex->texture != NULL,
> >       .u.has_sampler_deref = tex->sampler != NULL,
> > @@ -644,6 +646,7 @@ read_tex(read_ctx *ctx)
> >    tex->is_array = packed.u.is_array;
> >    tex->is_shadow = packed.u.is_shadow;
> >    tex->is_new_style_shadow = packed.u.is_new_style_shadow;
> > +   tex->is_image_sample_dref = packed.u.is_image_sample_dref;
> >    tex->component = packed.u.component;
> > 
> >    read_dest(ctx, &tex->dest, &tex->instr);
> > --
> > 2.14.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
> 
> 


More information about the mesa-dev mailing list