[Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.
Paul Berry
stereotype441 at gmail.com
Fri Dec 16 10:44:58 PST 2011
On 16 December 2011 10:04, Paul Berry <stereotype441 at gmail.com> wrote:
> On 15 December 2011 15:20, Kenneth Graunke <kenneth at whitecape.org> wrote:
>
>> Signed-off-by: Kenneth Graunke <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.
> /* 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
>> 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>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111216/dc23612e/attachment.html>
More information about the mesa-dev
mailing list