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

[Francesco Ansanelli]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160804/4c5086bf/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: c_enums.py
Type: text/x-python
Size: 1020 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160804/4c5086bf/attachment.py>