[RFC,2/2] nir/lower_tex: Update ->sampler_dim value before calling get_texture_size()

Submitted by Boris Brezillon on June 17, 2019, 10:21 a.m.

Details

Message ID 20190617102133.15346-3-boris.brezillon@collabora.com
State New
Headers show
Series "nir/lower_tex: Fix progress report when called in a loop" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Boris Brezillon June 17, 2019, 10:21 a.m.
get_texture_size() will create a txs instruction with ->sampler_dim set
to the original tex->sampler_dim. The condition to call lower_rect()
only checks the value of ->sampler_dim and whether lower_rect is
requested or not. This leads to an infinite loop when calling
nir_lower_tex() with the same options until it returns false.

In order to avoid that, let's move the tex->sampler_dim patching before
get_texture_size() is called. This way the txs instruction will have
->sampler_dim set to GLSL_SAMPLER_DIM_2D and nir_lower_tex() won't try
to lower it on the subsequent passes.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Another option would be to not call lower_rect() when the instruction
is a txs op.
---
 src/compiler/nir/nir_lower_tex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
index 73b26dbdb238..1ee961dbb90c 100644
--- a/src/compiler/nir/nir_lower_tex.c
+++ b/src/compiler/nir/nir_lower_tex.c
@@ -266,6 +266,8 @@  lower_offset(nir_builder *b, nir_tex_instr *tex)
 static void
 lower_rect(nir_builder *b, nir_tex_instr *tex)
 {
+   tex->sampler_dim = GLSL_SAMPLER_DIM_2D;
+
    nir_ssa_def *txs = get_texture_size(b, tex);
    nir_ssa_def *scale = nir_frcp(b, txs);
 
@@ -280,8 +282,6 @@  lower_rect(nir_builder *b, nir_tex_instr *tex)
                             &tex->src[i].src,
                             nir_src_for_ssa(nir_fmul(b, coords, scale)));
    }
-
-   tex->sampler_dim = GLSL_SAMPLER_DIM_2D;
 }
 
 static void

Comments


On Mon, 17 Jun 2019 11:00:22 -0500
Jason Ekstrand <jason@jlekstrand.net> wrote:

> On Mon, Jun 17, 2019 at 5:21 AM Boris Brezillon <
> boris.brezillon@collabora.com> wrote:  
> 
> > get_texture_size() will create a txs instruction with ->sampler_dim set
> > to the original tex->sampler_dim. The condition to call lower_rect()
> > only checks the value of ->sampler_dim and whether lower_rect is
> > requested or not. This leads to an infinite loop when calling
> > nir_lower_tex() with the same options until it returns false.
> >
> > In order to avoid that, let's move the tex->sampler_dim patching before
> > get_texture_size() is called. This way the txs instruction will have  
> > ->sampler_dim set to GLSL_SAMPLER_DIM_2D and nir_lower_tex() won't try  
> > to lower it on the subsequent passes.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> > Another option would be to not call lower_rect() when the instruction
> > is a txs op.
> > ---
> >  src/compiler/nir/nir_lower_tex.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/compiler/nir/nir_lower_tex.c
> > b/src/compiler/nir/nir_lower_tex.c
> > index 73b26dbdb238..1ee961dbb90c 100644
> > --- a/src/compiler/nir/nir_lower_tex.c
> > +++ b/src/compiler/nir/nir_lower_tex.c
> > @@ -266,6 +266,8 @@ lower_offset(nir_builder *b, nir_tex_instr *tex)
> >  static void
> >  lower_rect(nir_builder *b, nir_tex_instr *tex)
> >  {
> > +   tex->sampler_dim = GLSL_SAMPLER_DIM_2D;
> >  
> 
> Mind throwing in a short comment?
> 
> /* Set the sampler_dim to 2D here so that get_texture_size picks up the
> right dimensionality */

Will add that comment in v2.

> 
> With that,
> 
> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

Note that I don't have write permissions to the mesa tree, so I'll need
someone to apply the patch once it's considered ready to be merged.

> 
> 
> > +
> >     nir_ssa_def *txs = get_texture_size(b, tex);
> >     nir_ssa_def *scale = nir_frcp(b, txs);
> >
> > @@ -280,8 +282,6 @@ lower_rect(nir_builder *b, nir_tex_instr *tex)
> >                              &tex->src[i].src,
> >                              nir_src_for_ssa(nir_fmul(b, coords, scale)));
> >     }
> > -
> > -   tex->sampler_dim = GLSL_SAMPLER_DIM_2D;
> >  }
> >
> >  static void
> > --
> > 2.20.1
> >
> >