On Tuesday, December 08, 2015 01:46:23 PM Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs.h | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 106
> ++++++++++++++-----------------
> src/mesa/drivers/dri/i965/brw_nir.c | 45 ++++++++++---
> 3 files changed, 84 insertions(+), 69 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index bca4589..b55589f 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -278,7 +278,7 @@ public:
> unsigned stream_id);
> void emit_gs_thread_end();
> void emit_gs_input_load(const fs_reg &dst, const nir_src &vertex_src,
> - const fs_reg &indirect_offset, unsigned
> imm_offset,
> + unsigned base_offset, const nir_src &offset_src,
> unsigned num_components);
> void emit_cs_terminate();
> fs_reg *emit_cs_local_invocation_id_setup();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 3754622..18c9d65 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1603,29 +1603,31 @@ fs_visitor::emit_gs_vertex(const nir_src
> &vertex_count_nir_src,
> void
> fs_visitor::emit_gs_input_load(const fs_reg &dst,
> const nir_src &vertex_src,
> - const fs_reg &indirect_offset,
> - unsigned imm_offset,
> + unsigned base_offset,
> + const nir_src &offset_src,
> unsigned num_components)
> {
> struct brw_gs_prog_data *gs_prog_data = (struct brw_gs_prog_data *)
> prog_data;
>
> + nir_const_value *vertex_const = nir_src_as_const_value(vertex_src);
> + nir_const_value *offset_const = nir_src_as_const_value(offset_src);
> + const unsigned push_reg_count = gs_prog_data->base.urb_read_length * 8;
> +
> /* Offset 0 is the VUE header, which contains VARYING_SLOT_LAYER [.y],
> * VARYING_SLOT_VIEWPORT [.z], and VARYING_SLOT_PSIZ [.w]. Only
> * gl_PointSize is available as a GS input, however, so it must be that.
> */
> - const bool is_point_size =
> - indirect_offset.file == BAD_FILE && imm_offset == 0;
> -
> - nir_const_value *vertex_const = nir_src_as_const_value(vertex_src);
> - const unsigned push_reg_count = gs_prog_data->base.urb_read_length * 8;
> + const bool is_point_size = (base_offset == 0);
>
> - if (indirect_offset.file == BAD_FILE && vertex_const != NULL &&
> - 4 * imm_offset < push_reg_count) {
> - imm_offset = 4 * imm_offset + vertex_const->u[0] * push_reg_count;
> + if (offset_const != NULL && vertex_const != NULL &&
> + 4 * offset_const->u[0] < push_reg_count) {
This needs to be
4 * (base_offset + offset_const->u[0]) < push_reg_count
otherwise we use the push model for inputs that aren't actually pushed,
which breaks around 1500 piglit tests with INTEL_SCALAR_GS=1.
> + int imm_offset = (base_offset + offset_const->u[0]) * 4 +
> + vertex_const->u[0] * push_reg_count;
> /* This input was pushed into registers. */
> if (is_point_size) {
> /* gl_PointSize comes in .w */
> - bld.MOV(dst, fs_reg(ATTR, imm_offset + 3, dst.type));
> + assert(imm_offset == 0);
> + bld.MOV(dst, fs_reg(ATTR, 3, dst.type));
This is wrong. imm_offset contains the offset derived from the vertex
index, and gl_PointSize exists for multiple vertices. So it won't be 0.
Please just remove this change, putting it back to:
bld.MOV(dst, fs_reg(ATTR, imm_offset + 3, dst.type));
That fixes assertion failures in 3 Piglit tests.
With that fixed, patches 1-8 and 11-12 are:
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
