[Mesa-dev] [PATCH 3/5] intel: emit is_indexed_draw in the same VE than gl_DrawID

Jason Ekstrand jason at jlekstrand.net
Mon Apr 30 22:40:18 UTC 2018


On Sat, Apr 28, 2018 at 5:09 AM, Antia Puentes <apuentes at igalia.com> wrote:

> The Vertex Elements are now:
> * VE 1: <BaseVertex/firstvertex, BaseInstance, VertexID, InstanceID>
> * VE 2: <DrawID, is-indexed-draw, 0, 0>
>
> VE1 is it kept as it was before, VE2 additionally contains the new
> system value.
> ---
>  src/intel/compiler/brw_fs_nir.cpp             |  2 ++
>  src/intel/compiler/brw_nir.c                  | 11 +++++--
>  src/intel/compiler/brw_vec4.cpp               | 14 +++++----
>  src/mesa/drivers/dri/i965/brw_context.h       | 31 +++++++++++++++-----
>  src/mesa/drivers/dri/i965/brw_draw.c          | 21 +++++++++-----
>  src/mesa/drivers/dri/i965/brw_draw_upload.c   |  8 ++---
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 42
> +++++++++++++--------------
>  7 files changed, 80 insertions(+), 49 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 9698a0111ef..22beb0e00d1 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -116,6 +116,7 @@ emit_system_values_block(nir_block *block, fs_visitor
> *v)
>
>        case nir_intrinsic_load_vertex_id_zero_base:
>        case nir_intrinsic_load_base_vertex:
> +      case nir_intrinsic_load_is_indexed_draw:
>        case nir_intrinsic_load_first_vertex:
>        case nir_intrinsic_load_instance_id:
>        case nir_intrinsic_load_base_instance:
> @@ -2460,6 +2461,7 @@ fs_visitor::nir_emit_vs_intrinsic(const fs_builder
> &bld,
>     }
>
>     case nir_intrinsic_load_first_vertex:
> +   case nir_intrinsic_load_is_indexed_draw:
>        unreachable("lowered by brw_nir_lower_vs_inputs");
>
>     default:
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 16b0d86814f..a624deb6d2a 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -266,6 +266,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
>              case nir_intrinsic_load_base_instance:
>              case nir_intrinsic_load_vertex_id_zero_base:
>              case nir_intrinsic_load_instance_id:
> +            case nir_intrinsic_load_is_indexed_draw:
>              case nir_intrinsic_load_draw_id: {
>                 b.cursor = nir_after_instr(&intrin->instr);
>
> @@ -293,11 +294,15 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
>                    nir_intrinsic_set_component(load, 3);
>                    break;
>                 case nir_intrinsic_load_draw_id:
> -                  /* gl_DrawID is stored right after gl_VertexID and
> friends
> -                   * if any of them exist.
> +               case nir_intrinsic_load_is_indexed_draw:
> +                  /* gl_DrawID and IsIndexedDraw are stored right after
> +                   * gl_VertexID and friends if any of them exist.
>                     */
>                    nir_intrinsic_set_base(load, num_inputs + has_sgvs);
> -                  nir_intrinsic_set_component(load, 0);
> +                  if (intrin->intrinsic == nir_intrinsic_load_draw_id)
> +                     nir_intrinsic_set_component(load, 0);
> +                  else
> +                     nir_intrinsic_set_component(load, 1);
>                    break;
>                 default:
>                    unreachable("Invalid system value intrinsic");
> diff --git a/src/intel/compiler/brw_vec4.cpp
> b/src/intel/compiler/brw_vec4.cpp
> index e583c549204..898df90225f 100644
> --- a/src/intel/compiler/brw_vec4.cpp
> +++ b/src/intel/compiler/brw_vec4.cpp
> @@ -2833,6 +2833,13 @@ brw_compile_vs(const struct brw_compiler *compiler,
> void *log_data,
>        nr_attribute_slots++;
>     }
>
> +   /* gl_DrawID and IsIndexedDraw share its very own vec4 */
> +   if (shader->info.system_values_read &
> +       (BITFIELD64_BIT(SYSTEM_VALUE_DRAW_ID) |
> +        BITFIELD64_BIT(SYSTEM_VALUE_IS_INDEXED_DRAW))) {
> +      nr_attribute_slots++;
> +   }
> +
>     if (shader->info.system_values_read &
>         BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX))
>        prog_data->uses_basevertex = true;
> @@ -2857,12 +2864,9 @@ brw_compile_vs(const struct brw_compiler *compiler,
> void *log_data,
>         BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID))
>        prog_data->uses_instanceid = true;
>
> -   /* gl_DrawID has its very own vec4 */
>     if (shader->info.system_values_read &
> -       BITFIELD64_BIT(SYSTEM_VALUE_DRAW_ID)) {
> -      prog_data->uses_drawid = true;
> -      nr_attribute_slots++;
> -   }
> +       BITFIELD64_BIT(SYSTEM_VALUE_DRAW_ID))
> +          prog_data->uses_drawid = true;
>
>     /* The 3DSTATE_VS documentation lists the lower bound on "Vertex URB
> Entry
>      * Read Length" as 1 in vec4 mode, and 0 in SIMD8 mode.  Empirically,
> in
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 1e6a45eee1f..be43eab43cc 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -900,20 +900,35 @@ struct brw_context
>        } params;
>
>        /**
> -       * Buffer and offset used for GL_ARB_shader_draw_parameters
> -       * (for now, only gl_BaseVertex).
> +       * Buffer and offset used for GL_ARB_shader_draw_parameters which
> will
> +       * point to the indirect buffer for indirect draw calls.
>         */
>        struct brw_bo *draw_params_bo;
>        uint32_t draw_params_offset;
>
> +      struct {
> +         /**
> +          * The value of gl_DrawID for the current _mesa_prim. This
> always comes
> +          * in from it's own vertex buffer since it's not part of the
> indirect
> +          * draw parameters.
> +          */
> +         int gl_drawid;
> +
> +         /**
> +          * Stores if the current _mesa_prim is an indexed or non-indexed
> draw
> +          * (~0/0). Useful to calculate gl_BaseVertex as an AND of
> firstvertex
> +          * and is_indexed_draw.
> +          */
> +         int is_indexed_draw;
> +      } derived_params;
> +
>        /**
> -       * The value of gl_DrawID for the current _mesa_prim. This always
> comes
> -       * in from it's own vertex buffer since it's not part of the
> indirect
> -       * draw parameters.
> +       * Buffer and offset used for GL_ARB_shader_draw_parameters which
> contains
> +       * parameters that are not present in the indirect buffer. They
> will go in
> +       * their own vertex element.
>         */
> -      int gl_drawid;
> -      struct brw_bo *draw_id_bo;
> -      uint32_t draw_id_offset;
> +      struct brw_bo *derived_draw_params_bo;
> +      uint32_t derived_draw_params_offset;
>
>        /**
>         * Pointer to the the buffer storing the indirect draw parameters.
> It
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index f51f083178e..09199c30453 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -861,17 +861,22 @@ brw_draw_single_prim(struct gl_context *ctx,
>     }
>
>     /* gl_DrawID always needs its own vertex buffer since it's not part of
> -    * the indirect parameter buffer. If the program uses gl_DrawID we need
> -    * to flag BRW_NEW_VERTICES. For the first iteration, we don't have
> -    * valid vs_prog_data, but we always flag BRW_NEW_VERTICES before
> -    * the loop.
> +    * the indirect parameter buffer. Same for is_indexed_draw, which
> shares
> +    * the buffer with gl_DrawID. If the program uses gl_DrawID, we need to
> +    * flag BRW_NEW_VERTICES. For the first iteration, we don't have valid
> +    * vs_prog_data, but we always flag BRW_NEW_VERTICES before the loop.
>      */
> -   brw->draw.gl_drawid = prim->draw_id;
> -   brw_bo_unreference(brw->draw.draw_id_bo);
> -   brw->draw.draw_id_bo = NULL;
> -   if (prim_id > 0 && vs_prog_data->uses_drawid)
> +   if (prim_id > 0 &&
> +       vs_prog_data->uses_drawid)
>

This doesn't need to be line-wrapped anymore.

Series is

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


>        brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
>
> +   brw->draw.derived_params.gl_drawid = prim->draw_id;
> +   brw->draw.derived_params.is_indexed_draw = prim->indexed ? ~0 : 0;
> +
> +   brw_bo_unreference(brw->draw.derived_draw_params_bo);
> +   brw->draw.derived_draw_params_bo = NULL;
> +   brw->draw.derived_draw_params_offset = 0;
> +
>     if (devinfo->gen < 6)
>        brw_set_prim(brw, prim);
>     else
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index 7573f780f23..55566a7de44 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -716,11 +716,11 @@ brw_prepare_shader_draw_parameters(struct
> brw_context *brw)
>                        &brw->draw.draw_params_offset);
>     }
>
> -   if (vs_prog_data->uses_drawid) {
> +   if (vs_prog_data->uses_drawid || vs_prog_data->uses_is_indexed_draw) {
>        brw_upload_data(&brw->upload,
> -                      &brw->draw.gl_drawid, sizeof(brw->draw.gl_drawid),
> 4,
> -                      &brw->draw.draw_id_bo,
> -                      &brw->draw.draw_id_offset);
> +                      &brw->draw.derived_params,
> sizeof(brw->draw.derived_params), 4,
> +                      &brw->draw.derived_draw_params_bo,
> +                      &brw->draw.derived_draw_params_offset);
>     }
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index 1a32c60ae34..093954054fc 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -539,16 +539,21 @@ genX(emit_vertices)(struct brw_context *brw)
>     }
>  #endif
>
> -   const bool uses_firstvertex =
> -      vs_prog_data->uses_basevertex || vs_prog_data->uses_firstvertex;
> +   const bool uses_draw_params =
> +      vs_prog_data->uses_firstvertex ||
> +      vs_prog_data->uses_basevertex ||
> +      vs_prog_data->uses_baseinstance;
> +
> +   const bool uses_derived_draw_params =
> +      vs_prog_data->uses_drawid ||
> +      vs_prog_data->uses_is_indexed_draw;
>
> -   const bool needs_sgvs_element = (uses_firstvertex ||
> -                                    vs_prog_data->uses_baseinstance ||
> +   const bool needs_sgvs_element = (uses_draw_params ||
>                                      vs_prog_data->uses_instanceid ||
>                                      vs_prog_data->uses_vertexid);
>
>     unsigned nr_elements =
> -      brw->vb.nr_enabled + needs_sgvs_element + vs_prog_data->uses_drawid;
> +      brw->vb.nr_enabled + needs_sgvs_element + uses_derived_draw_params;
>
>  #if GEN_GEN < 8
>     /* If any of the formats of vb.enabled needs more that one upload, we
> need
> @@ -588,11 +593,8 @@ genX(emit_vertices)(struct brw_context *brw)
>     }
>
>     /* Now emit 3DSTATE_VERTEX_BUFFERS and 3DSTATE_VERTEX_ELEMENTS
> packets. */
> -   const bool uses_draw_params =
> -      uses_firstvertex ||
> -      vs_prog_data->uses_baseinstance;
>     const unsigned nr_buffers = brw->vb.nr_buffers +
> -      uses_draw_params + vs_prog_data->uses_drawid;
> +      uses_draw_params + uses_derived_draw_params;
>
>     if (nr_buffers) {
>        assert(nr_buffers <= (GEN_GEN >= 6 ? 33 : 17));
> @@ -626,11 +628,11 @@ genX(emit_vertices)(struct brw_context *brw)
>                                               0 /* step rate */);
>        }
>
> -      if (vs_prog_data->uses_drawid) {
> +      if (uses_derived_draw_params) {
>           dw = genX(emit_vertex_buffer_state)(brw, dw, brw->vb.nr_buffers
> + 1,
> -                                             brw->draw.draw_id_bo,
> -                                             brw->draw.draw_id_offset,
> -                                             brw->draw.draw_id_bo->size,
> +                                             brw->draw.derived_draw_
> params_bo,
> +                                             brw->draw.derived_draw_
> params_offset,
> +                                             brw->draw.derived_draw_
> params_bo->size,
>                                               0 /* stride */,
>                                               0 /* step rate */);
>        }
> @@ -772,8 +774,7 @@ genX(emit_vertices)(struct brw_context *brw)
>        };
>
>  #if GEN_GEN >= 8
> -      if (uses_firstvertex ||
> -          vs_prog_data->uses_baseinstance) {
> +      if (uses_draw_params) {
>           elem_state.VertexBufferIndex = brw->vb.nr_buffers;
>           elem_state.SourceElementFormat = ISL_FORMAT_R32G32_UINT;
>           elem_state.Component0Control = VFCOMP_STORE_SRC;
> @@ -782,11 +783,10 @@ genX(emit_vertices)(struct brw_context *brw)
>  #else
>        elem_state.VertexBufferIndex = brw->vb.nr_buffers;
>        elem_state.SourceElementFormat = ISL_FORMAT_R32G32_UINT;
> -      if (uses_firstvertex)
> +      if (uses_draw_params) {
>           elem_state.Component0Control = VFCOMP_STORE_SRC;
> -
> -      if (vs_prog_data->uses_baseinstance)
>           elem_state.Component1Control = VFCOMP_STORE_SRC;
> +      }
>
>        if (vs_prog_data->uses_vertexid)
>           elem_state.Component2Control = VFCOMP_STORE_VID;
> @@ -799,13 +799,13 @@ genX(emit_vertices)(struct brw_context *brw)
>        dw += GENX(VERTEX_ELEMENT_STATE_length);
>     }
>
> -   if (vs_prog_data->uses_drawid) {
> +   if (uses_derived_draw_params) {
>        struct GENX(VERTEX_ELEMENT_STATE) elem_state = {
>           .Valid = true,
>           .VertexBufferIndex = brw->vb.nr_buffers + 1,
> -         .SourceElementFormat = ISL_FORMAT_R32_UINT,
> +         .SourceElementFormat = ISL_FORMAT_R32G32_UINT,
>           .Component0Control = VFCOMP_STORE_SRC,
> -         .Component1Control = VFCOMP_STORE_0,
> +         .Component1Control = VFCOMP_STORE_SRC,
>           .Component2Control = VFCOMP_STORE_0,
>           .Component3Control = VFCOMP_STORE_0,
>  #if GEN_GEN < 5
> --
> 2.14.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180430/05dae924/attachment-0001.html>


More information about the mesa-dev mailing list