[Mesa-dev] [PATCH] i965: Add gl_state_index casts for PATCH_VERTICES_IN

[Kenneth Graunke]

On Wednesday, February 14, 2018 10:17:07 AM PST Jason Ekstrand wrote:
> On Wed, Feb 14, 2018 at 10:12 AM, Kenneth Graunke <kenneth at whitecape.org>
> wrote:
> 
> > On Wednesday, February 14, 2018 6:05:22 AM PST Marek Olšák wrote:
> > > On Wed, Feb 14, 2018 at 7:57 AM, Kenneth Graunke <kenneth at whitecape.org>
> > wrote:
> > > > On Tuesday, February 13, 2018 2:57:07 PM PST Jason Ekstrand wrote:
> > > >> This fixes the build in clang
> > > >> ---
> > > >>  src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp | 3 ++-
> > > >>  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> > b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> > > >> index 10a4ff4..69da83a 100644
> > > >> --- a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> > > >> +++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> > > >> @@ -254,7 +254,8 @@ brw_nir_lower_patch_vertices_in_to_uniform(nir_shader
> > *nir)
> > > >>        gl_state_index16 tokens[STATE_LENGTH] = {
> > > >>           STATE_INTERNAL,
> > > >>           nir->info.stage == MESA_SHADER_TESS_CTRL ?
> > > >> -            STATE_TCS_PATCH_VERTICES_IN :
> > STATE_TES_PATCH_VERTICES_IN,
> > > >> +            (gl_state_index16)STATE_TCS_PATCH_VERTICES_IN :
> > > >> +            (gl_state_index16)STATE_TES_PATCH_VERTICES_IN,
> > > >>        };
> > > >>        var->num_state_slots = 1;
> > > >>        var->state_slots =
> > > >>
> > > >
> > > > This is fine, but I prefer your plan from IRC:
> > > >
> > > > 1. Add STATE_MAX_VALUE = 0xffff to the enum.
> > > > 2. Mark the enum PACKED.
> > > > 3. Drop gl_state_index16 again, since gl_state_index should now be the
> > > >    desired size, and also an actual enum so it follows the actual C++
> > > >    enum rules.
> > > >
> > > > I suppose the downside is that it could cause "case not handled in
> > > > switch" warnings...
> > >
> > > While I don't care too much about how gl_state_index is done,
> > > gl_state_index can also contain arbitrary 16-bit signed integer
> > > values, i.e. it's not strictly enum. And yes, I said "signed".
> > >
> > > Marek
> >
> > Oh. :(  I guess using short or int16_t is pretty reasonable then...
> >
> 
> Does that mean we're happy with this patch?

Yeah, sorry, should have been clearer:

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180214/94a0a174/attachment.sig>