[Mesa-dev] [PATCH 2/2] i965: use a cast to silence a signed/unsigned comparison warning

[Brian Paul]

On 10/19/2011 02:24 PM, Paul Berry wrote:
> On 19 October 2011 12:58, Brian Paul <brianp at vmware.com
> <mailto:brianp at vmware.com>> wrote:
>
>     On 10/19/2011 01:53 PM, Paul Berry wrote:
>
>         On 18 October 2011 18:07, Brian Paul <brian.e.paul at gmail.com
>         <mailto:brian.e.paul at gmail.com>
>         <mailto:brian.e.paul at gmail.com
>         <mailto:brian.e.paul at gmail.com>__>> wrote:
>
>             From: Brian Paul <brianp at vmware.com
>         <mailto:brianp at vmware.com> <mailto:brianp at vmware.com
>         <mailto:brianp at vmware.com>>>
>
>
>             ---
>               .../drivers/dri/i965/brw_vec4___reg_allocate.cpp     |
>           2 +-
>               1 files changed, 1 insertions(+), 1 deletions(-)
>
>             diff --git
>         a/src/mesa/drivers/dri/i965/__brw_vec4_reg_allocate.cpp
>             b/src/mesa/drivers/dri/i965/__brw_vec4_reg_allocate.cpp
>             index 1ace91f..6de7682 100644
>             --- a/src/mesa/drivers/dri/i965/__brw_vec4_reg_allocate.cpp
>             +++ b/src/mesa/drivers/dri/i965/__brw_vec4_reg_allocate.cpp
>             @@ -217,7 +217,7 @@ vec4_visitor::reg_allocate()
>                    int reg = ra_get_node_reg(g, i);
>
>                    hw_reg_mapping[i] = first_assigned_grf +
>             brw->vs.ra_reg_to_grf[reg];
>             -      prog_data->total_grf = MAX2(prog_data->total_grf,
>             +      prog_data->total_grf = MAX2((int) prog_data->total_grf,
>                                               hw_reg_mapping[i] +
>             virtual_grf_sizes[i]);
>
>         Since we're storing the result into an unsigned value, I'd
>         prefer to
>         fix the warning by converting the second argument of MAX2 to
>         GLuint
>         rather than converting the first argument of MAX2 to int.
>
>
>     I was wondering which way that should go.  The types in that
>     expression are:
>
>     GLuint total_grf;
>     int hw_reg_mapping[];
>
>     can hw_reg_mapping[] values be negative?
>
>     -Brian
>
>
> No, every element of hw_reg_mapping is a register number, and those go
> from zero up.  It's possible that a better fix would be to make
> hw_reg_mapping[] and and virtual_grf_sizes[] unsigned, but that might
> have more cascading effects than you want to deal with.  Eric, do you
> have an opinion?
>
> Aside: what compiler (and compiler options) are you using to see these
> warnings?  I don't get them with GCC.

I built with 'make linux-dri-debug'.  It uses -Wall but I don't think 
the autoconf method sets that flag.

gcc -c -I. -Iserver -I../../../../../include 
-I../../../../../src/egl/drivers/dri -I../../../../../src/egl/main 
-I../../../../../src/mapi -I../../../../../src/mesa 
-I../../../../../src/mesa/drivers/dri/common -I/usr/local/include 
-I/usr/local/include/libdrm     -I/usr/local/include 
-I/usr/local/include/libdrm   -Wall -O0 -g -fPIC -DDEBUG 
-D_POSIX_SOURCE -D_POSIX_C_SOURCE=199309L -D_SVID_SOURCE -D_BSD_SOURCE 
-D_GNU_SOURCE -DPTHREADS -DUSE_EXTERNAL_DXTN_LIB=1 -DIN_DRI_DRIVER 
-DGLX_DIRECT_RENDERING -DGLX_INDIRECT_RENDERING -DHAVE_ALIAS 
-DHAVE_POSIX_MEMALIGN -fno-strict-aliasing  -I../../../../../src 
-I../../../../../src/mesa/drivers/dri/intel brw_vec4_reg_allocate.cpp 
-o brw_vec4_reg_allocate.o
brw_vec4_reg_allocate.cpp: In member function ‘void 
brw::vec4_visitor::reg_allocate()’:
brw_vec4_reg_allocate.cpp:220: warning: comparison between signed and 
unsigned integer expressions

I general, I like to enable as many warnings as practical to catch 
things that might be errors w/ other compilers (like MSVC).  I used to 
use -ansi -pedantic too.

-Brian