Hello, Thanks for review. Please find my comments below:
On Fri, Nov 9, 2018 at 2:52 PM Eric Engestrom <eric.engest...@intel.com> wrote: > On Tuesday, 2018-09-11 15:42:05 +0300, asimiklit.w...@gmail.com wrote: > > From: Andrii Simiklit <andrii.simik...@globallogic.com> > > > > 1. nir/nir_lower_vars_to_ssa.c:691:21: warning: > > unused variable ‘var’ > > nir_variable *var = path->path[0]->var; > > > > 2. nir_types.cpp:558:31: warning: > > ‘elem_align’ may be used uninitialized in this function > > unsigned elem_size, elem_align; > > nir_types.cpp:558:20: warning: > > ‘elem_size’ may be used uninitialized in this function > > unsigned elem_size, elem_align; > > > > Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com> > > --- > > src/compiler/nir/nir_lower_vars_to_ssa.c | 2 +- > > src/compiler/nir_types.cpp | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c > b/src/compiler/nir/nir_lower_vars_to_ssa.c > > index cd679be..96de261 100644 > > --- a/src/compiler/nir/nir_lower_vars_to_ssa.c > > +++ b/src/compiler/nir/nir_lower_vars_to_ssa.c > > @@ -688,7 +688,7 @@ nir_lower_vars_to_ssa_impl(nir_function_impl *impl) > > nir_deref_path *path = &node->path; > > > > assert(path->path[0]->deref_type == nir_deref_type_var); > > - nir_variable *var = path->path[0]->var; > > + MAYBE_UNUSED nir_variable *var = path->path[0]->var; > > > > /* We don't build deref nodes for non-local variables */ > > assert(var->data.mode == nir_var_local); > > Sure, or you could simply remove the single-use extra local variable and > just fold it into the assert :) > I agree,will fix :) > > > diff --git a/src/compiler/nir_types.cpp b/src/compiler/nir_types.cpp > > index d24f094..d39d87b 100644 > > --- a/src/compiler/nir_types.cpp > > +++ b/src/compiler/nir_types.cpp > > @@ -542,7 +542,7 @@ glsl_get_natural_size_align_bytes(const struct > glsl_type *type, > > } > > > > case GLSL_TYPE_ARRAY: { > > - unsigned elem_size, elem_align; > > + unsigned elem_size = 0, elem_align = 0; > > Not really keen on these ones; it looks like your compiler is ignoring the > unreachable() in glsl_get_natural_size_align_bytes() and mis-computing the > possible code-paths, resulting in it wrongly printing a warning. > You are right when I added the 'default:' case to the switch the warning is disappeared: case GLSL_TYPE_INTERFACE: case GLSL_TYPE_FUNCTION: +default: unreachable("type does not have a natural size"); What do you think about such solution? Is it acceptable for you? > I'd prefer to keep these two hunks as is, so that real issue in > glsl_get_natural_size_align_bytes() would get flagged appropriately. > > > glsl_get_natural_size_align_bytes(type->fields.array, > > &elem_size, &elem_align); > > *align = elem_align; > > @@ -554,7 +554,7 @@ glsl_get_natural_size_align_bytes(const struct > glsl_type *type, > > *size = 0; > > *align = 0; > > for (unsigned i = 0; i < type->length; i++) { > > - unsigned elem_size, elem_align; > > + unsigned elem_size = 0, elem_align = 0; > > > glsl_get_natural_size_align_bytes(type->fields.structure[i].type, > > &elem_size, &elem_align); > > *align = MAX2(*align, elem_align); > > -- > > 2.7.4 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev