[Mesa-dev] [PATCH 1/3] freedreno/a2xx: fix comparison out of range warnings

[Rob Clark]

Thanks.. my expectation is that ~0 gets cast to the same enum on both
sides of the != so I *think* we should be good.

BR,
-R

On Sat, Aug 6, 2016 at 9:05 AM, Francesco Ansanelli <francians at gmail.com> wrote:
> Hello,
>
> I did more tests... and I didn't experience any error.
> I attach last version of the script.
>
> Cheers,
> Francesco
>
> 2016-08-04 19:50 GMT+02:00 Francesco Ansanelli <francians at gmail.com>:
>>
>> Hi Eric,
>>
>> after your question:
>>
>> [snip]
>> I expect, with enums having varying size, that this cast will
>> not always work.
>> [snip]
>>
>> I started a script for testing with variable enum sizes and compilers...
>> If does makes sense to you, I'll try to loop from 1 to n (suggestions?)
>> and build with gcc and clang (and others?) to find the case you're taking
>> about.
>> I'm also thinking about a negative test...
>> What do you think?
>>
>> Cheers,
>> Francesco
>>
>> 2016-08-01 14:10 GMT+02:00 Eric Engestrom <eric.engestrom at imgtec.com>:
>>>
>>> On Sat, Jul 30, 2016 at 09:49:57AM +0200, Francesco Ansanelli wrote:
>>> > Signed-off-by: Francesco Ansanelli <francians at gmail.com>
>>> > ---
>>> >  src/gallium/drivers/freedreno/a2xx/fd2_screen.c |    8 ++++----
>>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>>> >
>>> > diff --git a/src/gallium/drivers/freedreno/a2xx/fd2_screen.c
>>> > b/src/gallium/drivers/freedreno/a2xx/fd2_screen.c
>>> > index fe4849b..007b9e6 100644
>>> > --- a/src/gallium/drivers/freedreno/a2xx/fd2_screen.c
>>> > +++ b/src/gallium/drivers/freedreno/a2xx/fd2_screen.c
>>> > @@ -61,7 +61,7 @@ fd2_screen_is_format_supported(struct pipe_screen
>>> > *pscreen,
>>> >
>>> >       if ((usage & (PIPE_BIND_SAMPLER_VIEW |
>>> >                               PIPE_BIND_VERTEX_BUFFER)) &&
>>> > -                     (fd2_pipe2surface(format) != ~0u)) {
>>> > +                     (fd2_pipe2surface(format) != (enum
>>> > a2xx_sq_surfaceformat)~0)) {
>>>
>>> You said the compiler warning goes away, but is the condition guaranteed
>>> to hit? I expect, with enums having varying size, that this cast will
>>> not always work. I agree with Rob Herring, adding the error value to the
>>> enum is better.
>>>
>>> >               retval |= usage & (PIPE_BIND_SAMPLER_VIEW |
>>> >                               PIPE_BIND_VERTEX_BUFFER);
>>> >       }
>>> > @@ -70,7 +70,7 @@ fd2_screen_is_format_supported(struct pipe_screen
>>> > *pscreen,
>>> >                               PIPE_BIND_DISPLAY_TARGET |
>>> >                               PIPE_BIND_SCANOUT |
>>> >                               PIPE_BIND_SHARED)) &&
>>> > -                     (fd2_pipe2color(format) != ~0u)) {
>>> > +                     (fd2_pipe2color(format) != (enum
>>> > a2xx_colorformatx)~0)) {
>>> >               retval |= usage & (PIPE_BIND_RENDER_TARGET |
>>> >                               PIPE_BIND_DISPLAY_TARGET |
>>> >                               PIPE_BIND_SCANOUT |
>>> > @@ -78,12 +78,12 @@ fd2_screen_is_format_supported(struct pipe_screen
>>> > *pscreen,
>>> >       }
>>> >
>>> >       if ((usage & PIPE_BIND_DEPTH_STENCIL) &&
>>> > -                     (fd_pipe2depth(format) != ~0u)) {
>>> > +                     (fd_pipe2depth(format) != (enum
>>> > adreno_rb_depth_format)~0)) {
>>> >               retval |= PIPE_BIND_DEPTH_STENCIL;
>>> >       }
>>> >
>>> >       if ((usage & PIPE_BIND_INDEX_BUFFER) &&
>>> > -                     (fd_pipe2index(format) != ~0u)) {
>>> > +                     (fd_pipe2index(format) != (enum
>>> > pc_di_index_size)~0)) {
>>> >               retval |= PIPE_BIND_INDEX_BUFFER;
>>> >       }
>>> >
>>> > --
>>> > 1.7.9.5
>>> >
>>> > _______________________________________________
>>> > mesa-dev mailing list
>>> > mesa-dev at lists.freedesktop.org
>>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>