Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Paul Berry]

On 15 December 2011 15:20, Kenneth Graunke  wrote:

> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_context.c |1 +
>  src/mesa/drivers/dri/i965/brw_context.h |3 ++
>  src/mesa/drivers/dri/i965/brw_gs_emit.c |   10 
>  src/mesa/drivers/dri/i965/gen6_sol.c|   38
> +++
>  4 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 7d360b0..fd60853 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
> dd_function_table *functions )
>brw_init_queryobj_functions(functions);
>
>functions->PrepareExecBegin = brwPrepareExecBegin;
> +   functions->BeginTransformFeedback = brw_begin_transform_feedback;
>functions->EndTransformFeedback = brw_end_transform_feedback;
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 8e52488..20623d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(const struct
> gl_fragment_program *fprog);
>
>  /* gen6_sol.c */
>  void
> +brw_begin_transform_feedback(struct gl_context *ctx,
> +struct gl_transform_feedback_object *obj);
>

Argh, I fail at code review.  I found another problem: this function is
missing the argument "GLenum mode", which should come after "gl_context
ctx", causing the function pointer assignment in brw_context.c to generate
a warning.  It's a benign warning, because the function only uses the ctx
argument, and the calling conventions work out so that it gets passed in
the same way regardless of whether mode is present.  But for portability we
should make the function signatures match.


> +void
>  brw_end_transform_feedback(struct gl_context *ctx,
>struct gl_transform_feedback_object *obj);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> index 72d4eca..5dd3734 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> @@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c, struct
> brw_gs_prog_key *key,
>*/
>   brw_MOV(p, get_element_ud(c->reg.header, 5),
>   get_element_ud(c->reg.SVBI, 0));
> +
> +  /* Make sure that the buffers have enough room for all the
> vertices. */
> +  brw_ADD(p, get_element_ud(c->reg.temp, 0),
> +get_element_ud(c->reg.SVBI, 0), brw_imm_ud(num_verts));
> +  brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
> +get_element_ud(c->reg.temp, 0),
> +get_element_ud(c->reg.SVBI, 4));
> +  brw_IF(p, BRW_EXECUTE_1);
> +
>   /* For each vertex, generate code to output each varying using the
>* appropriate binding table entry.
>*/
> @@ -392,6 +401,7 @@ gen6_sol_program(struct brw_gs_compile *c, struct
> brw_gs_prog_key *key,
> get_element_ud(c->reg.header, 5), brw_imm_ud(1));
>  }
>   }
> +  brw_ENDIF(p);
>
>   /* Now, reinitialize the header register from R0 to restore the
> parts of
>* the register that we overwrote while streaming out transform
> feedback
> diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c
> b/src/mesa/drivers/dri/i965/gen6_sol.c
> index b11bce2..56d4a6a 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sol.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sol.c
> @@ -26,6 +26,7 @@
>  * Code to initialize the binding table entries used by transform feedback.
>  */
>
> +#include "main/macros.h"
>  #include "brw_context.h"
>  #include "intel_buffer_objects.h"
>  #include "intel_batchbuffer.h"
> @@ -101,6 +102,43 @@ const struct brw_tracked_state gen6_sol_surface = {
>  };
>
>  void
> +brw_begin_transform_feedback(struct gl_context *ctx,
> +struct gl_transform_feedback_object *obj)
> +{
> +   struct intel_context *intel = intel_context(ctx);
> +   const struct gl_shader_program *vs_prog =
> +  ctx->Shader.CurrentVertexProgram;
> +   const struct gl_transform_feedback_info *linked_xfb_info =
> +  &vs_prog->LinkedTransformFeedback;
> +   struct gl_transform_feedback_object *xfb_obj =
> +  ctx->TransformFeedback.CurrentObject;
> +
> +   unsigned max_index = 0x;
> +
> +   /* Compute the maximum number of vertices that we can write without
> +* overflowing any of the buffers currently being used for feedback.
> +*/
> +   for (int i = 0; i < MAX_FEEDBACK_ATTRIBS; ++i) {
> +  unsigned stride = linked_xfb_info->BufferStride[i];
> +
> +  /* Skip any inactive buffers, which have a stride of 0. */
> +  if (stride == 0)
> +continue;
> +
> +  unsigned max_for_this_buffer = xfb_obj->Size[i] / (4 * stride);
> +  

Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Paul Berry]

On 16 December 2011 11:09, Ian Romanick  wrote:

> On 12/16/2011 10:44 AM, Paul Berry wrote:
>
>> On 16 December 2011 10:04, Paul Berry > > wrote:
>>
>>On 15 December 2011 15:20, Kenneth Graunke >**> wrote:
>>
>>Signed-off-by: Kenneth Graunke >**>
>>
>>---
>>  src/mesa/drivers/dri/i965/brw_**context.c |1 +
>>  src/mesa/drivers/dri/i965/brw_**context.h |3 ++
>>  src/mesa/drivers/dri/i965/brw_**gs_emit.c |   10 
>>  src/mesa/drivers/dri/i965/**gen6_sol.c|   38
>>++**+
>>  4 files changed, 52 insertions(+), 0 deletions(-)
>>
>>diff --git a/src/mesa/drivers/dri/i965/**brw_context.c
>>b/src/mesa/drivers/dri/i965/**brw_context.c
>>index 7d360b0..fd60853 100644
>>--- a/src/mesa/drivers/dri/i965/**brw_context.c
>>+++ b/src/mesa/drivers/dri/i965/**brw_context.c
>>@@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
>>dd_function_table *functions )
>>brw_init_queryobj_functions(**functions);
>>
>>functions->PrepareExecBegin = brwPrepareExecBegin;
>>+   functions->**BeginTransformFeedback =
>>brw_begin_transform_feedback;
>>functions->**EndTransformFeedback =
>> brw_end_transform_feedback;
>>  }
>>
>>diff --git a/src/mesa/drivers/dri/i965/**brw_context.h
>>b/src/mesa/drivers/dri/i965/**brw_context.h
>>index 8e52488..20623d4 100644
>>--- a/src/mesa/drivers/dri/i965/**brw_context.h
>>+++ b/src/mesa/drivers/dri/i965/**brw_context.h
>>@@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(**const struct
>>gl_fragment_program *fprog);
>>
>>  /* gen6_sol.c */
>>  void
>>+brw_begin_transform_feedback(**struct gl_context *ctx,
>>+struct gl_transform_feedback_object
>>*obj);
>>+void
>>  brw_end_transform_feedback(**struct gl_context *ctx,
>>struct gl_transform_feedback_object
>>*obj);
>>
>>diff --git a/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>b/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>index 72d4eca..5dd3734 100644
>>--- a/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>+++ b/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>@@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c,
>>struct brw_gs_prog_key *key,
>>*/
>>   brw_MOV(p, get_element_ud(c->reg.header, 5),
>>   get_element_ud(c->reg.SVBI, 0));
>>+
>>+  /* Make sure that the buffers have enough room for all
>>the vertices. */
>>+  brw_ADD(p, get_element_ud(c->reg.temp, 0),
>>+get_element_ud(c->reg.SVBI, 0),
>>brw_imm_ud(num_verts));
>>+  brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
>>+get_element_ud(c->reg.temp, 0),
>>+get_element_ud(c->reg.SVBI, 4));
>>+  brw_IF(p, BRW_EXECUTE_1);
>>+
>>
>>
>> Whoops, one other correction.  There's an off-by-one error--this should
>> be BRW_CONDITIONAL_LE.
>>
>> For example, let's say we're outputting triangles, the transform
>> feedback buffer is large enough to hold 6 vertices, and we've output 3
>> vertices already.  In that case num_verts is 3, SVBI.0 is 3, and SVBI.4
>> is 6.  The above logic compares SVBI.0 + num_verts < SVBI.4 (6 < 6), so
>> it concludes that there isn't room for the second triangle.  It should
>> be computing SVBI.0 + num_verts <= SVBI.4 (6 <= 6), because there is
>> just barely room for the second triangle.
>>
>
> Do we have piglit test cases for these edge conditions?  If we don't, we
> need them. :)
>

This one was caught by piglit tests that I'm hoping to send to the list
today.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Ian Romanick]

On 12/16/2011 10:44 AM, Paul Berry wrote:

On 16 December 2011 10:04, Paul Berry mailto:stereotype...@gmail.com>> wrote:

On 15 December 2011 15:20, Kenneth Graunke mailto:kenn...@whitecape.org>> wrote:

Signed-off-by: Kenneth Graunke mailto:kenn...@whitecape.org>>
---
  src/mesa/drivers/dri/i965/brw_context.c |1 +
  src/mesa/drivers/dri/i965/brw_context.h |3 ++
  src/mesa/drivers/dri/i965/brw_gs_emit.c |   10 
  src/mesa/drivers/dri/i965/gen6_sol.c|   38
+++
  4 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 7d360b0..fd60853 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
dd_function_table *functions )
brw_init_queryobj_functions(functions);

functions->PrepareExecBegin = brwPrepareExecBegin;
+   functions->BeginTransformFeedback =
brw_begin_transform_feedback;
functions->EndTransformFeedback = brw_end_transform_feedback;
  }

diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 8e52488..20623d4 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(const struct
gl_fragment_program *fprog);

  /* gen6_sol.c */
  void
+brw_begin_transform_feedback(struct gl_context *ctx,
+struct gl_transform_feedback_object
*obj);
+void
  brw_end_transform_feedback(struct gl_context *ctx,
struct gl_transform_feedback_object
*obj);

diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c
b/src/mesa/drivers/dri/i965/brw_gs_emit.c
index 72d4eca..5dd3734 100644
--- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
@@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c,
struct brw_gs_prog_key *key,
*/
   brw_MOV(p, get_element_ud(c->reg.header, 5),
   get_element_ud(c->reg.SVBI, 0));
+
+  /* Make sure that the buffers have enough room for all
the vertices. */
+  brw_ADD(p, get_element_ud(c->reg.temp, 0),
+get_element_ud(c->reg.SVBI, 0),
brw_imm_ud(num_verts));
+  brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
+get_element_ud(c->reg.temp, 0),
+get_element_ud(c->reg.SVBI, 4));
+  brw_IF(p, BRW_EXECUTE_1);
+


Whoops, one other correction.  There's an off-by-one error--this should
be BRW_CONDITIONAL_LE.

For example, let's say we're outputting triangles, the transform
feedback buffer is large enough to hold 6 vertices, and we've output 3
vertices already.  In that case num_verts is 3, SVBI.0 is 3, and SVBI.4
is 6.  The above logic compares SVBI.0 + num_verts < SVBI.4 (6 < 6), so
it concludes that there isn't room for the second triangle.  It should
be computing SVBI.0 + num_verts <= SVBI.4 (6 <= 6), because there is
just barely room for the second triangle.


Do we have piglit test cases for these edge conditions?  If we don't, we 
need them. :)



   /* For each vertex, generate code to output each varying
using the
* appropriate binding table entry.
*/
@@ -392,6 +401,7 @@ gen6_sol_program(struct brw_gs_compile *c,
struct brw_gs_prog_key *key,
 get_element_ud(c->reg.header, 5),
brw_imm_ud(1));
  }
   }
+  brw_ENDIF(p);

   /* Now, reinitialize the header register from R0 to
restore the parts of
* the register that we overwrote while streaming out
transform feedback
diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c
b/src/mesa/drivers/dri/i965/gen6_sol.c
index b11bce2..56d4a6a 100644
--- a/src/mesa/drivers/dri/i965/gen6_sol.c
+++ b/src/mesa/drivers/dri/i965/gen6_sol.c
@@ -26,6 +26,7 @@
  * Code to initialize the binding table entries used by
transform feedback.
  */

+#include "main/macros.h"
  #include "brw_context.h"
  #include "intel_buffer_objects.h"
  #include "intel_batchbuffer.h"
@@ -101,6 +102,43 @@ const struct brw_tracked_state
gen6_sol_surface = {
  };

  void
+brw_begin_transform_feedback(struct gl_context *ctx,
+ 

Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Paul Berry]

On 16 December 2011 10:04, Paul Berry  wrote:

> On 15 December 2011 15:20, Kenneth Graunke  wrote:
>
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/brw_context.c |1 +
>>  src/mesa/drivers/dri/i965/brw_context.h |3 ++
>>  src/mesa/drivers/dri/i965/brw_gs_emit.c |   10 
>>  src/mesa/drivers/dri/i965/gen6_sol.c|   38
>> +++
>>  4 files changed, 52 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
>> b/src/mesa/drivers/dri/i965/brw_context.c
>> index 7d360b0..fd60853 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> @@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
>> dd_function_table *functions )
>>brw_init_queryobj_functions(functions);
>>
>>functions->PrepareExecBegin = brwPrepareExecBegin;
>> +   functions->BeginTransformFeedback = brw_begin_transform_feedback;
>>functions->EndTransformFeedback = brw_end_transform_feedback;
>>  }
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
>> b/src/mesa/drivers/dri/i965/brw_context.h
>> index 8e52488..20623d4 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>> @@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(const struct
>> gl_fragment_program *fprog);
>>
>>  /* gen6_sol.c */
>>  void
>> +brw_begin_transform_feedback(struct gl_context *ctx,
>> +struct gl_transform_feedback_object *obj);
>> +void
>>  brw_end_transform_feedback(struct gl_context *ctx,
>>struct gl_transform_feedback_object *obj);
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c
>> b/src/mesa/drivers/dri/i965/brw_gs_emit.c
>> index 72d4eca..5dd3734 100644
>> --- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
>> @@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c, struct
>> brw_gs_prog_key *key,
>>*/
>>   brw_MOV(p, get_element_ud(c->reg.header, 5),
>>   get_element_ud(c->reg.SVBI, 0));
>> +
>> +  /* Make sure that the buffers have enough room for all the
>> vertices. */
>> +  brw_ADD(p, get_element_ud(c->reg.temp, 0),
>> +get_element_ud(c->reg.SVBI, 0), brw_imm_ud(num_verts));
>> +  brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
>> +get_element_ud(c->reg.temp, 0),
>> +get_element_ud(c->reg.SVBI, 4));
>> +  brw_IF(p, BRW_EXECUTE_1);
>> +
>>
>
Whoops, one other correction.  There's an off-by-one error--this should be
BRW_CONDITIONAL_LE.

For example, let's say we're outputting triangles, the transform feedback
buffer is large enough to hold 6 vertices, and we've output 3 vertices
already.  In that case num_verts is 3, SVBI.0 is 3, and SVBI.4 is 6.  The
above logic compares SVBI.0 + num_verts < SVBI.4 (6 < 6), so it concludes
that there isn't room for the second triangle.  It should be computing
SVBI.0 + num_verts <= SVBI.4 (6 <= 6), because there is just barely room
for the second triangle.


>   /* For each vertex, generate code to output each varying using the
>>* appropriate binding table entry.
>>*/
>> @@ -392,6 +401,7 @@ gen6_sol_program(struct brw_gs_compile *c, struct
>> brw_gs_prog_key *key,
>> get_element_ud(c->reg.header, 5), brw_imm_ud(1));
>>  }
>>   }
>> +  brw_ENDIF(p);
>>
>>   /* Now, reinitialize the header register from R0 to restore the
>> parts of
>>* the register that we overwrote while streaming out transform
>> feedback
>> diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c
>> b/src/mesa/drivers/dri/i965/gen6_sol.c
>> index b11bce2..56d4a6a 100644
>> --- a/src/mesa/drivers/dri/i965/gen6_sol.c
>> +++ b/src/mesa/drivers/dri/i965/gen6_sol.c
>> @@ -26,6 +26,7 @@
>>  * Code to initialize the binding table entries used by transform
>> feedback.
>>  */
>>
>> +#include "main/macros.h"
>>  #include "brw_context.h"
>>  #include "intel_buffer_objects.h"
>>  #include "intel_batchbuffer.h"
>> @@ -101,6 +102,43 @@ const struct brw_tracked_state gen6_sol_surface = {
>>  };
>>
>>  void
>> +brw_begin_transform_feedback(struct gl_context *ctx,
>> +struct gl_transform_feedback_object *obj)
>> +{
>> +   struct intel_context *intel = intel_context(ctx);
>> +   const struct gl_shader_program *vs_prog =
>> +  ctx->Shader.CurrentVertexProgram;
>> +   const struct gl_transform_feedback_info *linked_xfb_info =
>> +  &vs_prog->LinkedTransformFeedback;
>> +   struct gl_transform_feedback_object *xfb_obj =
>> +  ctx->TransformFeedback.CurrentObject;
>> +
>> +   unsigned max_index = 0x;
>> +
>> +   /* Compute the maximum number of vertices that we can write without
>> +* overflowing any of the buffers currently being used for feedback.
>> +*/
>> +   for (int i = 0; i < MAX_FEEDBACK_ATTRIBS; ++i) {
>>
>
> Minor nit: MAX_FEEDBA

Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Paul Berry]

On 15 December 2011 15:20, Kenneth Graunke  wrote:

> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_context.c |1 +
>  src/mesa/drivers/dri/i965/brw_context.h |3 ++
>  src/mesa/drivers/dri/i965/brw_gs_emit.c |   10 
>  src/mesa/drivers/dri/i965/gen6_sol.c|   38
> +++
>  4 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 7d360b0..fd60853 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
> dd_function_table *functions )
>brw_init_queryobj_functions(functions);
>
>functions->PrepareExecBegin = brwPrepareExecBegin;
> +   functions->BeginTransformFeedback = brw_begin_transform_feedback;
>functions->EndTransformFeedback = brw_end_transform_feedback;
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 8e52488..20623d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(const struct
> gl_fragment_program *fprog);
>
>  /* gen6_sol.c */
>  void
> +brw_begin_transform_feedback(struct gl_context *ctx,
> +struct gl_transform_feedback_object *obj);
> +void
>  brw_end_transform_feedback(struct gl_context *ctx,
>struct gl_transform_feedback_object *obj);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> index 72d4eca..5dd3734 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> @@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c, struct
> brw_gs_prog_key *key,
>*/
>   brw_MOV(p, get_element_ud(c->reg.header, 5),
>   get_element_ud(c->reg.SVBI, 0));
> +
> +  /* Make sure that the buffers have enough room for all the
> vertices. */
> +  brw_ADD(p, get_element_ud(c->reg.temp, 0),
> +get_element_ud(c->reg.SVBI, 0), brw_imm_ud(num_verts));
> +  brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
> +get_element_ud(c->reg.temp, 0),
> +get_element_ud(c->reg.SVBI, 4));
> +  brw_IF(p, BRW_EXECUTE_1);
> +
>   /* For each vertex, generate code to output each varying using the
>* appropriate binding table entry.
>*/
> @@ -392,6 +401,7 @@ gen6_sol_program(struct brw_gs_compile *c, struct
> brw_gs_prog_key *key,
> get_element_ud(c->reg.header, 5), brw_imm_ud(1));
>  }
>   }
> +  brw_ENDIF(p);
>
>   /* Now, reinitialize the header register from R0 to restore the
> parts of
>* the register that we overwrote while streaming out transform
> feedback
> diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c
> b/src/mesa/drivers/dri/i965/gen6_sol.c
> index b11bce2..56d4a6a 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sol.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sol.c
> @@ -26,6 +26,7 @@
>  * Code to initialize the binding table entries used by transform feedback.
>  */
>
> +#include "main/macros.h"
>  #include "brw_context.h"
>  #include "intel_buffer_objects.h"
>  #include "intel_batchbuffer.h"
> @@ -101,6 +102,43 @@ const struct brw_tracked_state gen6_sol_surface = {
>  };
>
>  void
> +brw_begin_transform_feedback(struct gl_context *ctx,
> +struct gl_transform_feedback_object *obj)
> +{
> +   struct intel_context *intel = intel_context(ctx);
> +   const struct gl_shader_program *vs_prog =
> +  ctx->Shader.CurrentVertexProgram;
> +   const struct gl_transform_feedback_info *linked_xfb_info =
> +  &vs_prog->LinkedTransformFeedback;
> +   struct gl_transform_feedback_object *xfb_obj =
> +  ctx->TransformFeedback.CurrentObject;
> +
> +   unsigned max_index = 0x;
> +
> +   /* Compute the maximum number of vertices that we can write without
> +* overflowing any of the buffers currently being used for feedback.
> +*/
> +   for (int i = 0; i < MAX_FEEDBACK_ATTRIBS; ++i) {
>

Minor nit: MAX_FEEDBACK_ATTRIBS (32) is the correct bound for generic Mesa
code, but in i965, we only support 4 buffers, so this loop bound should
probably be BRW_MAX_SOL_BUFFERS.


> +  unsigned stride = linked_xfb_info->BufferStride[i];
> +
> +  /* Skip any inactive buffers, which have a stride of 0. */
> +  if (stride == 0)
> +continue;
> +
> +  unsigned max_for_this_buffer = xfb_obj->Size[i] / (4 * stride);
> +  max_index = MIN2(max_index, max_for_this_buffer);
> +   }
> +
> +   /* Initialize the SVBI 0 register to zero and set the maximum index. */
> +   BEGIN_BATCH(4);
> +   OUT_BATCH(_3DSTATE_GS_SVB_INDEX << 16 | (4 - 2));
> +   OUT_BATCH(0); /* SVBI 0 */
> +   OUT_BATCH(0);
> +   OUT_BATCH(max_index);
> 

[Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_context.c |1 +
 src/mesa/drivers/dri/i965/brw_context.h |3 ++
 src/mesa/drivers/dri/i965/brw_gs_emit.c |   10 
 src/mesa/drivers/dri/i965/gen6_sol.c|   38 +++
 4 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index 7d360b0..fd60853 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct 
dd_function_table *functions )
brw_init_queryobj_functions(functions);
 
functions->PrepareExecBegin = brwPrepareExecBegin;
+   functions->BeginTransformFeedback = brw_begin_transform_feedback;
functions->EndTransformFeedback = brw_end_transform_feedback;
 }
 
diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 8e52488..20623d4 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(const struct 
gl_fragment_program *fprog);
 
 /* gen6_sol.c */
 void
+brw_begin_transform_feedback(struct gl_context *ctx,
+struct gl_transform_feedback_object *obj);
+void
 brw_end_transform_feedback(struct gl_context *ctx,
struct gl_transform_feedback_object *obj);
 
diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c 
b/src/mesa/drivers/dri/i965/brw_gs_emit.c
index 72d4eca..5dd3734 100644
--- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
@@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c, struct 
brw_gs_prog_key *key,
*/
   brw_MOV(p, get_element_ud(c->reg.header, 5),
   get_element_ud(c->reg.SVBI, 0));
+
+  /* Make sure that the buffers have enough room for all the vertices. */
+  brw_ADD(p, get_element_ud(c->reg.temp, 0),
+get_element_ud(c->reg.SVBI, 0), brw_imm_ud(num_verts));
+  brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
+get_element_ud(c->reg.temp, 0),
+get_element_ud(c->reg.SVBI, 4));
+  brw_IF(p, BRW_EXECUTE_1);
+
   /* For each vertex, generate code to output each varying using the
* appropriate binding table entry.
*/
@@ -392,6 +401,7 @@ gen6_sol_program(struct brw_gs_compile *c, struct 
brw_gs_prog_key *key,
 get_element_ud(c->reg.header, 5), brw_imm_ud(1));
  }
   }
+  brw_ENDIF(p);
 
   /* Now, reinitialize the header register from R0 to restore the parts of
* the register that we overwrote while streaming out transform feedback
diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c 
b/src/mesa/drivers/dri/i965/gen6_sol.c
index b11bce2..56d4a6a 100644
--- a/src/mesa/drivers/dri/i965/gen6_sol.c
+++ b/src/mesa/drivers/dri/i965/gen6_sol.c
@@ -26,6 +26,7 @@
  * Code to initialize the binding table entries used by transform feedback.
  */
 
+#include "main/macros.h"
 #include "brw_context.h"
 #include "intel_buffer_objects.h"
 #include "intel_batchbuffer.h"
@@ -101,6 +102,43 @@ const struct brw_tracked_state gen6_sol_surface = {
 };
 
 void
+brw_begin_transform_feedback(struct gl_context *ctx,
+struct gl_transform_feedback_object *obj)
+{
+   struct intel_context *intel = intel_context(ctx);
+   const struct gl_shader_program *vs_prog =
+  ctx->Shader.CurrentVertexProgram;
+   const struct gl_transform_feedback_info *linked_xfb_info =
+  &vs_prog->LinkedTransformFeedback;
+   struct gl_transform_feedback_object *xfb_obj =
+  ctx->TransformFeedback.CurrentObject;
+
+   unsigned max_index = 0x;
+
+   /* Compute the maximum number of vertices that we can write without
+* overflowing any of the buffers currently being used for feedback.
+*/
+   for (int i = 0; i < MAX_FEEDBACK_ATTRIBS; ++i) {
+  unsigned stride = linked_xfb_info->BufferStride[i];
+
+  /* Skip any inactive buffers, which have a stride of 0. */
+  if (stride == 0)
+continue;
+
+  unsigned max_for_this_buffer = xfb_obj->Size[i] / (4 * stride);
+  max_index = MIN2(max_index, max_for_this_buffer);
+   }
+
+   /* Initialize the SVBI 0 register to zero and set the maximum index. */
+   BEGIN_BATCH(4);
+   OUT_BATCH(_3DSTATE_GS_SVB_INDEX << 16 | (4 - 2));
+   OUT_BATCH(0); /* SVBI 0 */
+   OUT_BATCH(0);
+   OUT_BATCH(max_index);
+   ADVANCE_BATCH();
+}
+
+void
 brw_end_transform_feedback(struct gl_context *ctx,
struct gl_transform_feedback_object *obj)
 {
-- 
1.7.7.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev