[Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.
Ian Romanick
idr at freedesktop.org
Fri Dec 16 11:09:34 PST 2011
On 12/16/2011 10:44 AM, Paul Berry wrote:
> On 16 December 2011 10:04, Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>> wrote:
>
> On 15 December 2011 15:20, Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at whitecape.org>> wrote:
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at 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,
> + 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 = 0xffffffff;
> +
> + /* 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,
> struct gl_transform_feedback_object
> *obj)
> {
> --
> 1.7.7.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> <mailto:mesa-dev at lists.freedesktop.org>
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> Other than that one minor comment, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
More information about the mesa-dev
mailing list