Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.
On 15 December 2011 15:20, Kenneth Graunke kenn...@whitecape.org wrote:
Signed-off-by: Kenneth Graunke 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);
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);
+ max_index = MIN2(max_index, max_for_this_buffer);
+ }
+
+ /* Initialize the SVBI 0 register
Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.
On 15 December 2011 15:20, Kenneth Graunke kenn...@whitecape.org wrote:
Signed-off-by: Kenneth Graunke 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);
+
/* 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);
+ ADVANCE_BATCH();
+}
+
+void
brw_end_transform_feedback(struct gl_context *ctx,
Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.
On 12/16/2011 10:44 AM, Paul Berry wrote:
On 16 December 2011 10:04, Paul Berry stereotype...@gmail.com
mailto:stereotype...@gmail.com wrote:
On 15 December 2011 15:20, Kenneth Graunke kenn...@whitecape.org
mailto:kenn...@whitecape.org wrote:
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
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
Re: [Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.
On 16 December 2011 11:09, Ian Romanick i...@freedesktop.org wrote: On 12/16/2011 10:44 AM, Paul Berry wrote: On 16 December 2011 10:04, Paul Berry stereotype...@gmail.com mailto:stereotype441@gmail.**com stereotype...@gmail.com wrote: On 15 December 2011 15:20, Kenneth Graunke kenn...@whitecape.org mailto:kenn...@whitecape.org** wrote: Signed-off-by: Kenneth Graunke kenn...@whitecape.org 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. :) 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
