[3/4] i965: avoid 'unused variable' and 'may be used uninitialized' warnings

Submitted by andrey simiklit on Sept. 11, 2018, 12:42 p.m.

Details

Message ID 1536669727-22009-4-git-send-email-asimiklit.work@gmail.com
State New
Headers show
Series "mesa: fix against several compilation warnings" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

andrey simiklit Sept. 11, 2018, 12:42 p.m.
From: Andrii Simiklit <andrii.simiklit@globallogic.com>

1. brw_blorp.c:1502:4: warning:
    ‘num_layers’ may be used uninitialized in this function
2. brw_blorp.c:1502:4: warning:
    ‘start_layer’ may be used uninitialized in this function
3. brw_blorp.c:1502:4: warning:
    ‘level’ may be used uninitialized in this function
4. brw_pipe_control.c:311:34: warning:
    unused variable ‘devinfo’
5. brw_program_binary.c:209:19: warning:
    unused variable ‘gen_size’
6. brw_program_binary.c:216:19: warning:
    unused variable ‘nir_size’
7. intel_mipmap_tree.c:1698:10: warning:
    ‘initial_state’ may be used uninitialized in this function

Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
---
 src/mesa/drivers/dri/i965/brw_blorp.c          | 2 +-
 src/mesa/drivers/dri/i965/brw_pipe_control.c   | 2 +-
 src/mesa/drivers/dri/i965/brw_program_binary.c | 4 ++--
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c  | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
index ad747e0..a6e0f02 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -1443,7 +1443,7 @@  brw_blorp_clear_depth_stencil(struct brw_context *brw,
    if (x0 == x1 || y0 == y1)
       return;
 
-   uint32_t level, start_layer, num_layers;
+   uint32_t level = 0, start_layer = 0, num_layers = 0;
    struct isl_surf isl_tmp[4];
    struct blorp_surf depth_surf, stencil_surf;
 
diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
index 122ac26..a3f521b 100644
--- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
+++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
@@ -308,7 +308,7 @@  brw_emit_depth_stall_flushes(struct brw_context *brw)
 void
 gen7_emit_vs_workaround_flush(struct brw_context *brw)
 {
-   const struct gen_device_info *devinfo = &brw->screen->devinfo;
+   MAYBE_UNUSED const struct gen_device_info *devinfo = &brw->screen->devinfo;
 
    assert(devinfo->gen == 7);
    brw_emit_pipe_control_write(brw,
diff --git a/src/mesa/drivers/dri/i965/brw_program_binary.c b/src/mesa/drivers/dri/i965/brw_program_binary.c
index db03332..1298d9e 100644
--- a/src/mesa/drivers/dri/i965/brw_program_binary.c
+++ b/src/mesa/drivers/dri/i965/brw_program_binary.c
@@ -206,14 +206,14 @@  brw_program_deserialize_driver_blob(struct gl_context *ctx,
          break;
       switch ((enum driver_cache_blob_part)part_type) {
       case GEN_PART: {
-         uint32_t gen_size = blob_read_uint32(&reader);
+         MAYBE_UNUSED uint32_t gen_size = blob_read_uint32(&reader);
          assert(!reader.overrun &&
                 (uintptr_t)(reader.end - reader.current) > gen_size);
          deserialize_gen_program(&reader, ctx, prog, stage);
          break;
       }
       case NIR_PART: {
-         uint32_t nir_size = blob_read_uint32(&reader);
+         MAYBE_UNUSED uint32_t nir_size = blob_read_uint32(&reader);
          assert(!reader.overrun &&
                 (uintptr_t)(reader.end - reader.current) > nir_size);
          const struct nir_shader_compiler_options *options =
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 3668135..31e8122 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1769,8 +1769,8 @@  intel_miptree_alloc_aux(struct brw_context *brw,
    assert(mt->aux_buf == NULL);
 
    /* Get the aux buf allocation parameters for this miptree. */
-   enum isl_aux_state initial_state;
-   uint8_t memset_value;
+   enum isl_aux_state initial_state = ISL_AUX_STATE_AUX_INVALID;
+   uint8_t memset_value = 0;
    struct isl_surf aux_surf;
    MAYBE_UNUSED bool aux_surf_ok = false;
 

Comments

On Tuesday, 2018-09-11 15:42:06 +0300, asimiklit.work@gmail.com wrote:
> From: Andrii Simiklit <andrii.simiklit@globallogic.com>
> 
> 1. brw_blorp.c:1502:4: warning:
>     ‘num_layers’ may be used uninitialized in this function
> 2. brw_blorp.c:1502:4: warning:
>     ‘start_layer’ may be used uninitialized in this function
> 3. brw_blorp.c:1502:4: warning:
>     ‘level’ may be used uninitialized in this function
> 4. brw_pipe_control.c:311:34: warning:
>     unused variable ‘devinfo’
> 5. brw_program_binary.c:209:19: warning:
>     unused variable ‘gen_size’
> 6. brw_program_binary.c:216:19: warning:
>     unused variable ‘nir_size’
> 7. intel_mipmap_tree.c:1698:10: warning:
>     ‘initial_state’ may be used uninitialized in this function
> 
> Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c          | 2 +-
>  src/mesa/drivers/dri/i965/brw_pipe_control.c   | 2 +-
>  src/mesa/drivers/dri/i965/brw_program_binary.c | 4 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c  | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
> index ad747e0..a6e0f02 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -1443,7 +1443,7 @@ brw_blorp_clear_depth_stencil(struct brw_context *brw,
>     if (x0 == x1 || y0 == y1)
>        return;
>  
> -   uint32_t level, start_layer, num_layers;
> +   uint32_t level = 0, start_layer = 0, num_layers = 0;

It would be cleaner to promote the assert() a few lines below into assume():
  assert((mask & BUFFER_BIT_DEPTH) || stencil_mask);

>     struct isl_surf isl_tmp[4];
>     struct blorp_surf depth_surf, stencil_surf;
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index 122ac26..a3f521b 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -308,7 +308,7 @@ brw_emit_depth_stall_flushes(struct brw_context *brw)
>  void
>  gen7_emit_vs_workaround_flush(struct brw_context *brw)
>  {
> -   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> +   MAYBE_UNUSED const struct gen_device_info *devinfo = &brw->screen->devinfo;
>  
>     assert(devinfo->gen == 7);
>     brw_emit_pipe_control_write(brw,
> diff --git a/src/mesa/drivers/dri/i965/brw_program_binary.c b/src/mesa/drivers/dri/i965/brw_program_binary.c
> index db03332..1298d9e 100644
> --- a/src/mesa/drivers/dri/i965/brw_program_binary.c
> +++ b/src/mesa/drivers/dri/i965/brw_program_binary.c
> @@ -206,14 +206,14 @@ brw_program_deserialize_driver_blob(struct gl_context *ctx,
>           break;
>        switch ((enum driver_cache_blob_part)part_type) {
>        case GEN_PART: {
> -         uint32_t gen_size = blob_read_uint32(&reader);
> +         MAYBE_UNUSED uint32_t gen_size = blob_read_uint32(&reader);
>           assert(!reader.overrun &&
>                  (uintptr_t)(reader.end - reader.current) > gen_size);
>           deserialize_gen_program(&reader, ctx, prog, stage);
>           break;
>        }
>        case NIR_PART: {
> -         uint32_t nir_size = blob_read_uint32(&reader);
> +         MAYBE_UNUSED uint32_t nir_size = blob_read_uint32(&reader);
>           assert(!reader.overrun &&
>                  (uintptr_t)(reader.end - reader.current) > nir_size);
>           const struct nir_shader_compiler_options *options =

R-b on this two MAYBE_UNUSED hunks.

> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 3668135..31e8122 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1769,8 +1769,8 @@ intel_miptree_alloc_aux(struct brw_context *brw,
>     assert(mt->aux_buf == NULL);
>  
>     /* Get the aux buf allocation parameters for this miptree. */
> -   enum isl_aux_state initial_state;
> -   uint8_t memset_value;
> +   enum isl_aux_state initial_state = ISL_AUX_STATE_AUX_INVALID;
> +   uint8_t memset_value = 0;

Again, maybe promote assert(aux_surf_ok); to assume(), but this just
would just hide issues if there ever were any added.

>     struct isl_surf aux_surf;
>     MAYBE_UNUSED bool aux_surf_ok = false;
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Fri, Nov 9, 2018 at 3:19 PM Eric Engestrom <eric.engestrom@intel.com>
wrote:

> On Tuesday, 2018-09-11 15:42:06 +0300, asimiklit.work@gmail.com wrote:
> > From: Andrii Simiklit <andrii.simiklit@globallogic.com>
> >
> > 1. brw_blorp.c:1502:4: warning:
> >     ‘num_layers’ may be used uninitialized in this function
> > 2. brw_blorp.c:1502:4: warning:
> >     ‘start_layer’ may be used uninitialized in this function
> > 3. brw_blorp.c:1502:4: warning:
> >     ‘level’ may be used uninitialized in this function
> > 4. brw_pipe_control.c:311:34: warning:
> >     unused variable ‘devinfo’
> > 5. brw_program_binary.c:209:19: warning:
> >     unused variable ‘gen_size’
> > 6. brw_program_binary.c:216:19: warning:
> >     unused variable ‘nir_size’
> > 7. intel_mipmap_tree.c:1698:10: warning:
> >     ‘initial_state’ may be used uninitialized in this function
> >
> > Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c          | 2 +-
> >  src/mesa/drivers/dri/i965/brw_pipe_control.c   | 2 +-
> >  src/mesa/drivers/dri/i965/brw_program_binary.c | 4 ++--
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c  | 4 ++--
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index ad747e0..a6e0f02 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -1443,7 +1443,7 @@ brw_blorp_clear_depth_stencil(struct brw_context
> *brw,
> >     if (x0 == x1 || y0 == y1)
> >        return;
> >
> > -   uint32_t level, start_layer, num_layers;
> > +   uint32_t level = 0, start_layer = 0, num_layers = 0;
>
> It would be cleaner to promote the assert() a few lines below into
> assume():
>   assert((mask & BUFFER_BIT_DEPTH) || stencil_mask);
>
>
I tried to reproduce this warnings but it is not reproducible for some
reason.
Possible due to upgrade from Ubuntu 16.04 to 18.04 (Maybe due to some GCC
changes) or some mesa config changes
I can't say for sure. So I would like to remove these changes from this
patch due to it.
Is it acceptable for you?


> >     struct isl_surf isl_tmp[4];
> >     struct blorp_surf depth_surf, stencil_surf;
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> > index 122ac26..a3f521b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> > +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> > @@ -308,7 +308,7 @@ brw_emit_depth_stall_flushes(struct brw_context *brw)
> >  void
> >  gen7_emit_vs_workaround_flush(struct brw_context *brw)
> >  {
> > -   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> > +   MAYBE_UNUSED const struct gen_device_info *devinfo =
> &brw->screen->devinfo;
> >
> >     assert(devinfo->gen == 7);
> >     brw_emit_pipe_control_write(brw,
> > diff --git a/src/mesa/drivers/dri/i965/brw_program_binary.c
> b/src/mesa/drivers/dri/i965/brw_program_binary.c
> > index db03332..1298d9e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_program_binary.c
> > +++ b/src/mesa/drivers/dri/i965/brw_program_binary.c
> > @@ -206,14 +206,14 @@ brw_program_deserialize_driver_blob(struct
> gl_context *ctx,
> >           break;
> >        switch ((enum driver_cache_blob_part)part_type) {
> >        case GEN_PART: {
> > -         uint32_t gen_size = blob_read_uint32(&reader);
> > +         MAYBE_UNUSED uint32_t gen_size = blob_read_uint32(&reader);
> >           assert(!reader.overrun &&
> >                  (uintptr_t)(reader.end - reader.current) > gen_size);
> >           deserialize_gen_program(&reader, ctx, prog, stage);
> >           break;
> >        }
> >        case NIR_PART: {
> > -         uint32_t nir_size = blob_read_uint32(&reader);
> > +         MAYBE_UNUSED uint32_t nir_size = blob_read_uint32(&reader);
> >           assert(!reader.overrun &&
> >                  (uintptr_t)(reader.end - reader.current) > nir_size);
> >           const struct nir_shader_compiler_options *options =
>
> R-b on this two MAYBE_UNUSED hunks.
>
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 3668135..31e8122 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1769,8 +1769,8 @@ intel_miptree_alloc_aux(struct brw_context *brw,
> >     assert(mt->aux_buf == NULL);
> >
> >     /* Get the aux buf allocation parameters for this miptree. */
> > -   enum isl_aux_state initial_state;
> > -   uint8_t memset_value;
> > +   enum isl_aux_state initial_state = ISL_AUX_STATE_AUX_INVALID;
> > +   uint8_t memset_value = 0;
>
> Again, maybe promote assert(aux_surf_ok); to assume(), but this just
> would just hide issues if there ever were any added.
>
>
The similar results for this change, I unable to reproduce it.
So I would like to remove it too )


> >     struct isl_surf aux_surf;
> >     MAYBE_UNUSED bool aux_surf_ok = false;
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>