<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 5, 2017 at 12:49 AM, Alejandro Piñeiro <span dir="ltr"><<a href="mailto:apinheiro@igalia.com" target="_blank">apinheiro@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 05/05/17 04:11, Jason Ekstrand wrote:<br>
> ---<br>
>  src/intel/compiler/brw_fs_nir.<wbr>cpp | 26 +++-----------------------<br>
>  src/intel/compiler/brw_nir.c      | 22 +++++++++++++++++++---<br>
>  2 files changed, 22 insertions(+), 26 deletions(-)<br>
><br>
> diff --git a/src/intel/compiler/brw_fs_<wbr>nir.cpp b/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
> index d4ce753..f408d48 100644<br>
> --- a/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
> +++ b/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
> @@ -1914,27 +1914,15 @@ fs_visitor::emit_gs_input_<wbr>load(const fs_reg &dst,<br>
>     nir_const_value *offset_const = nir_src_as_const_value(offset_<wbr>src);<br>
>     const unsigned push_reg_count = gs_prog_data->base.urb_read_<wbr>length * 8;<br>
><br>
> -   /* Offset 0 is the VUE header, which contains VARYING_SLOT_LAYER [.y],<br>
> -    * VARYING_SLOT_VIEWPORT [.z], and VARYING_SLOT_PSIZ [.w].  Only<br>
> -    * gl_PointSize is available as a GS input, however, so it must be that.<br>
> -    */<br>
> -   const bool is_point_size = (base_offset == 0);<br>
> -<br>
>     /* TODO: figure out push input layout for invocations == 1 */<br>
>     if (gs_prog_data->invocations == 1 &&<br>
>         offset_const != NULL && vertex_const != NULL &&<br>
>         4 * (base_offset + offset_const->u32[0]) < push_reg_count) {<br>
>        int imm_offset = (base_offset + offset_const->u32[0]) * 4 +<br>
>                         vertex_const->u32[0] * push_reg_count;<br>
> -      /* This input was pushed into registers. */<br>
> -      if (is_point_size) {<br>
> -         /* gl_PointSize comes in .w */<br>
> -         bld.MOV(dst, fs_reg(ATTR, imm_offset + 3, dst.type));<br>
> -      } else {<br>
> -         for (unsigned i = 0; i < num_components; i++) {<br>
> -            bld.MOV(offset(dst, bld, i),<br>
> -                    fs_reg(ATTR, imm_offset + i + first_component, dst.type));<br>
> -         }<br>
> +      for (unsigned i = 0; i < num_components; i++) {<br>
> +         bld.MOV(offset(dst, bld, i),<br>
> +                 fs_reg(ATTR, imm_offset + i + first_component, dst.type));<br>
>        }<br>
>        return;<br>
>     }<br>
> @@ -2104,14 +2092,6 @@ fs_visitor::emit_gs_input_<wbr>load(const fs_reg &dst,<br>
>           }<br>
>        }<br>
>     }<br>
> -<br>
> -   if (is_point_size) {<br>
> -      /* Read the whole VUE header (because of alignment) and read .w. */<br>
> -      fs_reg tmp = bld.vgrf(dst.type, 4);<br>
> -      inst->dst = tmp;<br>
> -      inst->size_written = 4 * REG_SIZE;<br>
> -      bld.MOV(dst, offset(tmp, bld, 3));<br>
> -   }<br>
>  }<br>
><br>
>  fs_reg<br>
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c<br>
> index 725143f..d92e8fa 100644<br>
> --- a/src/intel/compiler/brw_nir.c<br>
> +++ b/src/intel/compiler/brw_nir.c<br>
> @@ -363,9 +363,25 @@ brw_nir_lower_vue_inputs(nir_<wbr>shader *nir, bool is_scalar,<br>
><br>
>              if (intrin->intrinsic == nir_intrinsic_load_input ||<br>
>                  intrin->intrinsic == nir_intrinsic_load_per_vertex_<wbr>input) {<br>
> -               int vue_slot = vue_map->varying_to_slot[<wbr>intrin->const_index[0]];<br>
> -               assert(vue_slot != -1);<br>
> -               intrin->const_index[0] = vue_slot;<br>
> +               /* Offset 0 is the VUE header, which contains<br>
> +                * VARYING_SLOT_LAYER [.y], VARYING_SLOT_VIEWPORT [.z], and<br>
> +                * VARYING_SLOT_PSIZ [.w].  Only gl_PointSize is available as a<br>
> +                * GS input, however, so it must be that.<br>
<br>
</div></div>"must be that" sounds like something that can be asserted. With or<br>
without that:<br></blockquote><div><br></div><div>Actually, I should just drop that part of the comment.  I copy and pasted the comment from elsewhere and that part no longer makes sense.  I've got a local patch which adds support for reading LAYER and VIEWPORT by just adding them to the switch below.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewed-by: Alejandro Piñeiro <<a href="mailto:apinheiro@igalia.com">apinheiro@igalia.com</a>><br><div class="HOEnZb"><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 class="HOEnZb"><div class="h5">
> +                */<br>
> +               int varying = nir_intrinsic_base(intrin);<br>
> +               int vue_slot;<br>
> +               switch (varying) {<br>
> +               case VARYING_SLOT_PSIZ:<br>
> +                  nir_intrinsic_set_base(intrin, 0);<br>
> +                  nir_intrinsic_set_component(<wbr>intrin, 3);<br>
> +                  break;<br>
> +<br>
> +               default:<br>
> +                  vue_slot = vue_map->varying_to_slot[<wbr>varying];<br>
> +                  assert(vue_slot != -1);<br>
> +                  nir_intrinsic_set_base(intrin, vue_slot);<br>
> +                  break;<br>
> +               }<br>
>              }<br>
>           }<br>
>        }<br>
<br>
</div></div></blockquote></div><br></div></div>