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

[Jordan Justen]

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