Re: [Mesa-dev] [PATCH v3 4/8] intel: Handle firstvertex in an identical way to BaseVertex

Tue, 27 Mar 2018 07:53:19 -0700 

Thanks for looking at this.

On 27/03/18 03:06, Ian Romanick wrote:


It looks like there are a couple places in brw_fs_nir.cpp
(emit_system_values_block and fs_visitor::nir_emit_vs_intrinsic) that
handle nir_intrinsic_load_base_instance but not
nir_intrinsic_load_first_vertex.  Should those cases be added?
We should definitely include it in the emit_system_values_block to mark it as unreachable, 
as like the others, it is lowered by brw_nir_lower_vs_inputs.
About fs_visitor::nir_emit_vs_intrinsic, it makes sense to add it there too. As far as I can see, the vs_inputs intrinsics should be lowered by then,  we may want to replace the code by an 
"unreachable (lowered by brw_nir_lower_vs_inputs)".


I have pushed a branch to my fd.o repo called gl_VertexID-fixes that
rebases this series on current master.  I had to apply some fixes to the
last patch in the series to get it to build.  I have tested that whole
series patch-by-patch in the CI with no regressions.  I have also
applied my R-b on several patches in the series.  I think once we decide
what to do about this patch and the squash! patch in my branch, we can
(finally) push this.

Excellent.


On 01/25/2018 10:15 AM, Antia Puentes wrote:

Until we set gl_BaseVertex to zero for non-indexed draw calls
both have an identical value.

The Vertex Elements are kept like that:
* VE 1: <BaseVertex/firstvertex, BaseInstance, VertexID, InstanceID>
* VE 2: <Draw ID, 0, 0, 0>
---
  src/intel/compiler/brw_nir.c                  |  3 +++
  src/intel/compiler/brw_vec4.cpp               |  1 +
  src/mesa/drivers/dri/i965/brw_context.h       |  8 ++++++--
  src/mesa/drivers/dri/i965/brw_draw.c          | 14 +++++++++-----
  src/mesa/drivers/dri/i965/brw_draw_upload.c   |  7 +++++--
  src/mesa/drivers/dri/i965/genX_state_upload.c | 11 +++++++----
  6 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index dbddef0d04d..34b1e44adf0 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -239,6 +239,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
     const bool has_sgvs =
        nir->info.system_values_read &
        (BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX) |
+       BITFIELD64_BIT(SYSTEM_VALUE_FIRST_VERTEX) |
         BITFIELD64_BIT(SYSTEM_VALUE_BASE_INSTANCE) |
         BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
         BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID));
@@ -261,6 +262,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
switch (intrin->intrinsic) { 
              case nir_intrinsic_load_base_vertex:
+            case nir_intrinsic_load_first_vertex:
              case nir_intrinsic_load_base_instance:
              case nir_intrinsic_load_vertex_id_zero_base:
              case nir_intrinsic_load_instance_id:
@@ -278,6 +280,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
                 nir_intrinsic_set_base(load, num_inputs);
                 switch (intrin->intrinsic) {
                 case nir_intrinsic_load_base_vertex:
+               case nir_intrinsic_load_first_vertex:
                    nir_intrinsic_set_component(load, 0);
                    break;
                 case nir_intrinsic_load_base_instance:
diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
index 36e17d77d47..06c79630119 100644
--- a/src/intel/compiler/brw_vec4.cpp
+++ b/src/intel/compiler/brw_vec4.cpp
@@ -2788,6 +2788,7 @@ brw_compile_vs(const struct brw_compiler *compiler, void 
*log_data,
      */
     if (shader->info.system_values_read &
         (BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX) |
+        BITFIELD64_BIT(SYSTEM_VALUE_FIRST_VERTEX) |
          BITFIELD64_BIT(SYSTEM_VALUE_BASE_INSTANCE) |
          BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
          BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID))) {
diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 9046acd175c..0a20706567e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -881,8 +881,12 @@ struct brw_context
struct { 
        struct {
-         /** The value of gl_BaseVertex for the current _mesa_prim. */
-         int gl_basevertex;
+         /**
+          * Either the value of gl_BaseVertex for indexed draw calls or the
+          * value of the argument <first> for non-indexed draw calls for the
+          * current _mesa_prim.
+          */
+         int firstvertex;
/** The value of gl_BaseInstance for the current _mesa_prim. */ 
           int gl_baseinstance;
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
b/src/mesa/drivers/dri/i965/brw_draw.c
index 50cf8b12c74..a1a5161fd35 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -816,25 +816,29 @@ brw_draw_single_prim(struct gl_context *ctx,
      * always flag if the shader uses one of the values. For direct draws,
      * we only flag if the values change.
      */
-   const int new_basevertex =
+   const int new_firstvertex =
        prim->indexed ? prim->basevertex : prim->start;
     const int new_baseinstance = prim->base_instance;
     const struct brw_vs_prog_data *vs_prog_data =
        brw_vs_prog_data(brw->vs.base.prog_data);
     if (prim_id > 0) {
-      const bool uses_draw_parameters =
+      const bool uses_firstvertex =
           vs_prog_data->uses_basevertex ||
+         vs_prog_data->uses_firstvertex;
+
+      const bool uses_draw_parameters =
+         uses_firstvertex ||
           vs_prog_data->uses_baseinstance;
if ((uses_draw_parameters && prim->is_indirect) || 
-          (vs_prog_data->uses_basevertex &&
-           brw->draw.params.gl_basevertex != new_basevertex) ||
+          (uses_firstvertex &&
+           brw->draw.params.firstvertex != new_firstvertex) ||
            (vs_prog_data->uses_baseinstance &&
             brw->draw.params.gl_baseinstance != new_baseinstance))
           brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
     }
- brw->draw.params.gl_basevertex = new_basevertex; 
+   brw->draw.params.firstvertex = new_firstvertex;
     brw->draw.params.gl_baseinstance = new_baseinstance;
     brw_bo_unreference(brw->draw.draw_params_bo);
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c 
index 9b81999ea05..52bf752bff9 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -696,8 +696,11 @@ brw_prepare_shader_draw_parameters(struct brw_context *brw)
     const struct brw_vs_prog_data *vs_prog_data =
        brw_vs_prog_data(brw->vs.base.prog_data);
- /* For non-indirect draws, upload gl_BaseVertex. */ 
-   if ((vs_prog_data->uses_basevertex || vs_prog_data->uses_baseinstance) &&
+   const bool uses_firstvertex =
+      vs_prog_data->uses_basevertex || vs_prog_data->uses_firstvertex;
+
+   /* For non-indirect draws, upload the shader draw parameters */
+   if ((uses_firstvertex || vs_prog_data->uses_baseinstance) &&
         brw->draw.draw_params_bo == NULL) {
        intel_upload_data(brw, &brw->draw.params, sizeof(brw->draw.params), 4,
                        &brw->draw.draw_params_bo,
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index d0a980f9730..2f2a573fd05 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -488,7 +488,10 @@ genX(emit_vertices)(struct brw_context *brw)
     }
  #endif
- const bool needs_sgvs_element = (vs_prog_data->uses_basevertex || 
+   const bool uses_firstvertex =
+      vs_prog_data->uses_basevertex || vs_prog_data->uses_firstvertex;
+
+   const bool needs_sgvs_element = (uses_firstvertex ||
                                      vs_prog_data->uses_baseinstance ||
                                      vs_prog_data->uses_instanceid ||
                                      vs_prog_data->uses_vertexid);
@@ -533,7 +536,7 @@ genX(emit_vertices)(struct brw_context *brw)
/* Now emit 3DSTATE_VERTEX_BUFFERS and 3DSTATE_VERTEX_ELEMENTS packets. */ 
     const bool uses_draw_params =
-      vs_prog_data->uses_basevertex ||
+      uses_firstvertex ||
        vs_prog_data->uses_baseinstance;
     const unsigned nr_buffers = brw->vb.nr_buffers +
        uses_draw_params + vs_prog_data->uses_drawid;
@@ -711,7 +714,7 @@ genX(emit_vertices)(struct brw_context *brw)
        };
#if GEN_GEN >= 8 
-      if (vs_prog_data->uses_basevertex ||
+      if (uses_firstvertex ||
            vs_prog_data->uses_baseinstance) {
           elem_state.VertexBufferIndex = brw->vb.nr_buffers;
           elem_state.SourceElementFormat = (enum GENX(SURFACE_FORMAT)) 
ISL_FORMAT_R32G32_UINT;
@@ -721,7 +724,7 @@ genX(emit_vertices)(struct brw_context *brw)
  #else
        elem_state.VertexBufferIndex = brw->vb.nr_buffers;
        elem_state.SourceElementFormat = (enum GENX(SURFACE_FORMAT)) 
ISL_FORMAT_R32G32_UINT;
-      if (vs_prog_data->uses_basevertex)
+      if (uses_firstvertex)
           elem_state.Component0Control = VFCOMP_STORE_SRC;
if (vs_prog_data->uses_baseinstance) 





_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to