[Mesa-dev] [PATCH] i965/fs: Restore compute shader support in brw_nir_lower_inputs

[Kenneth Graunke]

On Tuesday, October 13, 2015 09:02:48 PM Jordan Justen wrote:
> On 2015-10-13 20:04:36, Kenneth Graunke wrote:
> > On Tuesday, October 13, 2015 01:44:55 PM Jordan Justen wrote:
> > > The commit shown below caused compute shaders to hit the unreachable
> > > in the default of the switch block. Restore compute shaders to use the
> > > fragment shader path.
> > > 
> > > Also, simplify the fragment/compute path to only support scalar mode.
> > > 
> > > commit 2953c3d76178d7589947e6ea1dbd902b7b02b3d4
> > > Author: Kenneth Graunke <kenneth at whitecape.org>
> > > Date:   Fri Aug 14 15:15:11 2015 -0700
> > > 
> > >     i965/vs: Map scalar VS input locations properly; avoid tons of MOVs.
> > > 
> > > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > > Cc: Kenneth Graunke <kenneth at whitecape.org>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_nir.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
> > > index 4f35d81..357ee4f 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > > @@ -96,8 +96,10 @@ brw_nir_lower_inputs(nir_shader *nir, bool is_scalar)
> > >        }
> > >        break;
> > >     case MESA_SHADER_FRAGMENT:
> > > +   case MESA_SHADER_COMPUTE:
> > > +      assert(is_scalar);
> > >        nir_assign_var_locations(&nir->inputs, &nir->num_inputs,
> > > -                               is_scalar ? type_size_scalar : type_size_vec4);
> > > +                               type_size_scalar);
> > >        break;
> > >     default:
> > >        unreachable("unsupported shader stage");
> > > 
> > 
> > I didn't think compute shaders had inputs...so it might make more sense
> > just to do:
> > 
> >    case MESA_SHADER_COMPUTE:
> >       /* Compute shaders have no inputs. */
> >       break;
> 
> Huh. I looked at nir_assign_var_locations and it seemed to ignore
> UBO's and SSBO's, but not normal uniforms. So, I thought it might be
> needed for uniforms.
> 
> But, I didn't see a regression after skipping calling it. Can you
> confirm it is not needed for uniforms?
> 
> Strange, it looks like fs_visitor::nir_setup_uniforms will use
> var->data.driver_location which nir_assign_var_locations sets. So, I'm
> wondering why not calling it is not causing trouble.
> 
> > Either way, sorry for breaking this, and...
> > Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> No problem. Hitting unreachable is easy to debug. :)
> 
> -Jordan

brw_nir_lower_inputs() / brw_nir_lower_outputs() are just for shader
inputs/outputs.  Uniforms are handled by the caller one line below.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151014/ba4dbfd7/attachment-0001.sig>