[Mesa-dev] [PATCH V4 2/6] glsl: assign hidden uniforms their slot id earlier

[Timothy Arceri]

On Mon, 2015-10-12 at 01:06 +0200, Marek Olšák wrote:
> On Sun, Oct 11, 2015 at 9:20 AM, Timothy Arceri <
> t_arceri at yahoo.com.au> wrote:
> > On Sat, 2015-10-10 at 18:06 +0200, Marek Olšák wrote:
> > > Hi Timothy,
> > > 
> > > One of these 3 commits breaks compilation for Talos shaders with
> > > gallium. My piglit patch "glsl-1.30/sampler-bug: ..." contains a
> > > minimal test case. I can't say which commit, because Mesa fails
> > > to
> > > build between them.
> > >  It has something to do with uniforms, structures,
> > > and samplers.
> > > 
> > > commit dcd9cd03837545055ce2a315e7e8840cc3254d1a
> > > Author: Timothy Arceri <t_arceri at yahoo.com.au>
> > > Date:   Sun Aug 30 12:50:34 2015 +1000
> > > 
> > >     glsl: store uniform slot id in var location field
> > > 
> > 
> > Hi Marek,
> > 
> > The piglit test passes on my intel based laptop I can't test on
> > anything else until later this week (as I'm traveling) but my guess
> > is
> > its something to do with the above patch.
> > 
> > The other two patches shouldn't change anything for gallium drivers
> > "glsl: assign hidden uniforms their slot id earlier" just assigns
> > hidden uniforms their slot id earlier and there shouldn't be any
> > difference once the IR gets to glsl_to_tgsi.
> > 
> > Also "glsl: order indices for samplers inside a struct array"
> > shouldn't
> > change things either as in your test the sampler are not inside the
> > struct.
> > 
> > There is some code in the glsl_to_tgsi pass that looks like the
> > location field would have always been -1 for uniforms other the UBO
> > and
> > UBO members maybe this has something to do with the problem now
> > that
> > all uniforms now get a non -1 value.
> > 
> >       case ir_var_uniform:
> >          entry = new(mem_ctx) variable_storage(var,
> > PROGRAM_UNIFORM,
> >                                                var->data.location);
> >          this->variables.push_tail(entry);
> >          break;
> > 
> > I hope this helps get you started. If you haven't figured it out by
> > later in the week than I'll take a look on my desktop once I get
> > home.
> 
> The problem is that _mesa_get_sampler_uniform_value returns a sampler
> index which is greater than 16. If I allow large sampler indices, it
> starts to fail with sampler.file==TGSI_FILE_NULL in the TGSI codegen
> later. I didn't investigate further.
> 
> Marek


So from my bisect run your test was passing until:

a907b5dd162b7911b8c21f6d54837831bc078059 is the first bad commit
commit a907b5dd162b7911b8c21f6d54837831bc078059
Author: Marek Olšák <marek.olsak at amd.com>
Date:   Mon Oct 5 03:26:48 2015 +0200

    st/mesa: translate fragment shaders into TGSI when we get them
    
    Reviewed-by: Dave Airlie <airlied at redhat.com>
    Reviewed-by: Brian Paul <brianp at vmware.com>
    Tested-by: Brian Paul <brianp at vmware.com>