<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Mar 26, 2016 at 8:48 AM, Rob Clark <span dir="ltr"><<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Mar 25, 2016 at 7:12 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> ---<br>
>  src/compiler/nir/nir.h                     |  3 ++<br>
>  src/compiler/nir/nir_lower_system_values.c | 74 ++++++++++++++++++++++++++++--<br>
>  2 files changed, 74 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h<br>
> index 907dc34..d46858e 100644<br>
> --- a/src/compiler/nir/nir.h<br>
> +++ b/src/compiler/nir/nir.h<br>
> @@ -1678,6 +1678,9 @@ typedef struct nir_shader_compiler_options {<br>
>      * are simulated by floats.)<br>
>      */<br>
>     bool native_integers;<br>
> +<br>
> +   /* Indicates that the driver only has zero-based vertex id */<br>
> +   bool vertex_id_zero_based;<br>
<br>
</span>btw, mind setting that to true in ir3_nir.c or reminding me when you<br>
push this so that I can?  (From quick look, I think it should be false<br>
for vc4..)<br><div><div class="h5"></div></div></blockquote><div><br></div><div>Done.  I also threw setting the flag for i965 into this patch.  I was going to do that on its own, but in here seems reasonable.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
>  } nir_shader_compiler_options;<br>
><br>
>  typedef struct nir_shader_info {<br>
> diff --git a/src/compiler/nir/nir_lower_system_values.c b/src/compiler/nir/nir_lower_system_values.c<br>
> index 2bd787d..c1cd139 100644<br>
> --- a/src/compiler/nir/nir_lower_system_values.c<br>
> +++ b/src/compiler/nir/nir_lower_system_values.c<br>
> @@ -55,9 +55,77 @@ convert_block(nir_block *block, void *void_state)<br>
><br>
>        b->cursor = nir_after_instr(&load_var->instr);<br>
><br>
> -      nir_intrinsic_op sysval_op =<br>
> -         nir_intrinsic_from_system_value(var->data.location);<br>
> -      nir_ssa_def *sysval = nir_load_system_value(b, sysval_op, 0);<br>
> +      nir_ssa_def *sysval;<br>
> +      switch (var->data.location) {<br>
> +      case SYSTEM_VALUE_GLOBAL_INVOCATION_ID: {<br>
> +         /* From the GLSL man page for gl_GlobalInvocationID:<br>
> +          *<br>
> +          *    "The value of gl_GlobalInvocationID is equal to<br>
> +          *    gl_WorkGroupID * gl_WorkGroupSize + gl_LocalInvocationID"<br>
> +          */<br>
> +<br>
> +         nir_const_value local_size;<br>
> +         local_size.u32[0] = b->shader->info.cs.local_size[0];<br>
> +         local_size.u32[1] = b->shader->info.cs.local_size[1];<br>
> +         local_size.u32[2] = b->shader->info.cs.local_size[2];<br>
> +<br>
> +         nir_ssa_def *group_id =<br>
> +            nir_load_system_value(b, nir_intrinsic_load_work_group_id, 0);<br>
> +         nir_ssa_def *local_id =<br>
> +            nir_load_system_value(b, nir_intrinsic_load_local_invocation_id, 0);<br>
> +<br>
> +         sysval = nir_iadd(b, nir_imul(b, group_id,<br>
> +                                          nir_build_imm(b, 3, local_size)),<br>
> +                              local_id);<br>
> +         break;<br>
> +      }<br>
> +<br>
> +      case SYSTEM_VALUE_LOCAL_INVOCATION_INDEX: {<br>
> +         /* From the GLSL man page for gl_LocalInvocationIndex:<br>
> +          *<br>
> +          *    ?The value of gl_LocalInvocationIndex is equal to<br>
<br>
</div></div>looks like that ? was supposed to be a "<span class=""><br></span></blockquote><div><br></div><div>Yup.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +          *    gl_LocalInvocationID.z * gl_WorkGroupSize.x *<br>
> +          *    gl_WorkGroupSize.y + gl_LocalInvocationID.y *<br>
> +          *    gl_WorkGroupSize.x + gl_LocalInvocationID.x"<br>
> +          */<br>
> +         nir_ssa_def *local_id =<br>
> +            nir_load_system_value(b, nir_intrinsic_load_local_invocation_id, 0);<br>
> +<br>
> +         unsigned stride_y = b->shader->info.cs.local_size[0];<br>
> +         unsigned stride_z = b->shader->info.cs.local_size[0] *<br>
> +                             b->shader->info.cs.local_size[1];<br>
> +<br>
> +         sysval = nir_iadd(b, nir_imul(b, nir_channel(b, local_id, 2),<br>
> +                                          nir_imm_int(b, stride_z)),<br>
> +                              nir_iadd(b, nir_imul(b, nir_channel(b, local_id, 1),<br>
> +                                                      nir_imm_int(b, stride_y)),<br>
> +                                          nir_channel(b, local_id, 0)));<br>
<br>
</span>so this does get big enough to be difficult to read and the<br>
stride_y/stride_z confused me ;-)<br>
<br>
Not sure if something like this is an improvement:<br>
<br>
        nir_ssa_def *size_x = nir_imm_int(b, b->shader->info.cs.local_size[0]);<br>
        nir_ssa_def *size_y = nir_imm_int(b, b->shader->info.cs.local_size[1]);<br>
<br>
        /* gl_LocalInvocationID.z * gl_WorkGroupSize.x * gl_WorkGroupSize.y */<br>
        sysval = nir_imul(nir_imul(b, nir_channel(b, local_id, 2),<br>
size_x), size_y);<br>
        /* + gl_LocalInvocationID.y * gl_WorkGroupSize.x */<br>
        sysval = nir_iadd(b, sysval, nir_imul(b, nir_channel(b,<br>
local_id, 1), size_x));<br>
        /* + gl_LocalInvocationID.x */<br>
        sysval = nir_iadd(b, sysval, nir_channel(b, local_id, 0));<br>
<br>
either way, I guess you don't need to pre-multiply size.x and size.y,<br>
const prop pass will handle that, and not doing so makes it easier to<br>
map back to the formula from the man page.  And if you make them<br>
nir_ssa_def's instead of unsigned then you could make the "math" part<br>
of it less busy.<br></blockquote><div><br></div><div>I like that much better.  I updated the patch accordingly (but without the extra comments).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
anyways, with those small comments,<br>
<br>
Reviewed-by: Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>><br><div><div class="h5"></div></div></blockquote><div><br></div><div>Thanks!<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
> +         break;<br>
> +      }<br>
> +<br>
> +      case SYSTEM_VALUE_VERTEX_ID:<br>
> +         if (b->shader->options->vertex_id_zero_based) {<br>
> +            sysval = nir_iadd(b,<br>
> +               nir_load_system_value(b, nir_intrinsic_load_vertex_id_zero_base, 0),<br>
> +               nir_load_system_value(b, nir_intrinsic_load_base_vertex, 0));<br>
> +         } else {<br>
> +            sysval = nir_load_system_value(b, nir_intrinsic_load_vertex_id, 0);<br>
> +         }<br>
> +         break;<br>
> +<br>
> +      case SYSTEM_VALUE_INSTANCE_INDEX:<br>
> +         sysval = nir_iadd(b,<br>
> +            nir_load_system_value(b, nir_intrinsic_load_instance_id, 0),<br>
> +            nir_load_system_value(b, nir_intrinsic_load_base_instance, 0));<br>
> +         break;<br>
> +<br>
> +      default: {<br>
> +         nir_intrinsic_op sysval_op =<br>
> +            nir_intrinsic_from_system_value(var->data.location);<br>
> +         sysval = nir_load_system_value(b, sysval_op, 0);<br>
> +         break;<br>
> +      } /* default */<br>
> +      }<br>
><br>
>        nir_ssa_def_rewrite_uses(&load_var->dest.ssa, nir_src_for_ssa(sysval));<br>
>        nir_instr_remove(&load_var->instr);<br>
> --<br>
> 2.5.0.400.gff86faf<br>
><br>
</div></div>> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>