[Mesa-dev] [PATCH 13/21] i965: Resolve GCC sign-compare warning.

[Rhys Kidd]

Hello Ian,

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.

Either way, there were no piglit regressions on ILK.

Will push as part of a V2 of this patchset.

On 6 August 2015 at 02:07, Ian Romanick <idr at freedesktop.org> wrote:

> On 08/02/2015 11:09 PM, Rhys Kidd wrote:
> > mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c: In function
> 'brw_prepare_vertices':
> > mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c:434:22: warning:
> comparison between signed and unsigned integer expressions [-Wsign-compare]
> >     for (i = j = 0; i < brw->vb.nr_enabled; i++) {
> >                       ^
> > mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c:557:17: warning:
> comparison between signed and unsigned integer expressions [-Wsign-compare]
> >    for (i = 0; i < nr_uploads; i++) {
> >                  ^
> > mesa/src/mesa/drivers/dri/i965/brw_draw_upload.c:569:18: warning:
> comparison between signed and unsigned integer expressions [-Wsign-compare]
> >     for (i = 0; i < nr_uploads; i++) {
> >                   ^
> >
> > Signed-off-by: Rhys Kidd <rhyskidd at gmail.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_draw_upload.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> > index c6dd69b..ffd5373 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> > @@ -392,10 +392,10 @@ brw_prepare_vertices(struct brw_context *brw)
> >     /* BRW_NEW_VS_PROG_DATA */
> >     GLbitfield64 vs_inputs = brw->vs.prog_data->inputs_read;
> >     const unsigned char *ptr = NULL;
> > -   GLuint interleaved = 0;
> > +   GLuint interleaved = 0, i, j;
> >     unsigned int min_index = brw->vb.min_index + brw->basevertex;
> >     unsigned int max_index = brw->vb.max_index + brw->basevertex;
> > -   int delta, i, j;
> > +   int delta;
> >
> >     struct brw_vertex_element *upload[VERT_ATTRIB_MAX];
> >     GLuint nr_uploads = 0;
> > @@ -418,7 +418,7 @@ brw_prepare_vertices(struct brw_context *brw)
> >     /* Accumulate the list of enabled arrays. */
> >     brw->vb.nr_enabled = 0;
> >     while (vs_inputs) {
> > -      GLuint i = ffsll(vs_inputs) - 1;
> > +      i = ffsll(vs_inputs) - 1;
>
> This is surprising.  Is the other "i" used?  There is some potential for
> this to be a functional change.
>
> >        struct brw_vertex_element *input = &brw->vb.inputs[i];
> >
> >        vs_inputs &= ~BITFIELD64_BIT(i);
> > @@ -438,7 +438,7 @@ brw_prepare_vertices(struct brw_context *brw)
> >        if (_mesa_is_bufferobj(glarray->BufferObj)) {
> >        struct intel_buffer_object *intel_buffer =
> >           intel_buffer_object(glarray->BufferObj);
> > -      int k;
> > +      GLuint k;
> >
> >        /* If we have a VB set to be uploaded for this buffer object
> >         * already, reuse that VB state so that we emit fewer
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150806/5bcba66a/attachment.html>