<div dir="ltr"><div><div>Hello Ian,<br><br></div>The other "i" is used within a for() loop below in that function. On review, I am inclined to resolve the underlying issue being shadowing a name within a broader scope, by changing the variable name of the GLuint.<br><br></div><div>Either way, there were no piglit regressions on ILK.<br></div><div><br></div>Will push as part of a V2 of this patchset.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 6 August 2015 at 02:07, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 08/02/2015 11:09 PM, Rhys Kidd wrote:<br>
> mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c: In function 'brw_prepare_vertices':<br>
> mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c:434:22: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]<br>
>     for (i = j = 0; i < brw->vb.nr_enabled; i++) {<br>
>                       ^<br>
> mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c:557:17: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]<br>
>    for (i = 0; i < nr_uploads; i++) {<br>
>                  ^<br>
> mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c:569:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]<br>
>     for (i = 0; i < nr_uploads; i++) {<br>
>                   ^<br>
><br>
> Signed-off-by: Rhys Kidd <<a href="mailto:rhyskidd@gmail.com">rhyskidd@gmail.com</a>><br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_draw_upload.c | 8 ++++----<br>
>  1 file changed, 4 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c<br>
> index c6dd69b..ffd5373 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c<br>
> @@ -392,10 +392,10 @@ brw_prepare_vertices(struct brw_context *brw)<br>
>     /* BRW_NEW_VS_PROG_DATA */<br>
>     GLbitfield64 vs_inputs = brw->vs.prog_data->inputs_read;<br>
>     const unsigned char *ptr = NULL;<br>
> -   GLuint interleaved = 0;<br>
> +   GLuint interleaved = 0, i, j;<br>
>     unsigned int min_index = brw->vb.min_index + brw->basevertex;<br>
>     unsigned int max_index = brw->vb.max_index + brw->basevertex;<br>
> -   int delta, i, j;<br>
> +   int delta;<br>
><br>
>     struct brw_vertex_element *upload[VERT_ATTRIB_MAX];<br>
>     GLuint nr_uploads = 0;<br>
> @@ -418,7 +418,7 @@ brw_prepare_vertices(struct brw_context *brw)<br>
>     /* Accumulate the list of enabled arrays. */<br>
>     brw->vb.nr_enabled = 0;<br>
>     while (vs_inputs) {<br>
> -      GLuint i = ffsll(vs_inputs) - 1;<br>
> +      i = ffsll(vs_inputs) - 1;<br>
<br>
</div></div>This is surprising.  Is the other "i" used?  There is some potential for<br>
this to be a functional change.<br>
<div class="HOEnZb"><div class="h5"><br>
>        struct brw_vertex_element *input = &brw->vb.inputs[i];<br>
><br>
>        vs_inputs &= ~BITFIELD64_BIT(i);<br>
> @@ -438,7 +438,7 @@ brw_prepare_vertices(struct brw_context *brw)<br>
>        if (_mesa_is_bufferobj(glarray->BufferObj)) {<br>
>        struct intel_buffer_object *intel_buffer =<br>
>           intel_buffer_object(glarray->BufferObj);<br>
> -      int k;<br>
> +      GLuint k;<br>
><br>
>        /* If we have a VB set to be uploaded for this buffer object<br>
>         * already, reuse that VB state so that we emit fewer<br>
><br>
<br>
</div></div></blockquote></div><br></div>