Re: [Mesa-dev] [PATCH] ac/nir: silence maybe-uninitialized warnings

[Nicolai Hähnle]

On 28.08.2017 10:30, Nicolai Hähnle wrote:
On 27.08.2017 23:43, Grazvydas Ignotas wrote:
These are likely false positives, but are also annoying because they
show up on every "make install", which causes ac_nir_to_llvm to be
rebuilt here. Initializing those variables to NULL should be harmless
even when unnecessary.
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
  src/amd/common/ac_nir_to_llvm.c | 15 +++++++++------
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c 
b/src/amd/common/ac_nir_to_llvm.c
index 664d83b..823ea42 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1332,11 +1332,11 @@ static LLVMValueRef emit_i2b(struct 
ac_llvm_context *ctx,
  static LLVMValueRef emit_f2f16(struct nir_to_llvm_context *ctx,
                     LLVMValueRef src0)
  {
      LLVMValueRef result;
-    LLVMValueRef cond;
+    LLVMValueRef cond = NULL;
      src0 = to_float(&ctx->ac, src0);
      result = LLVMBuildFPTrunc(ctx->builder, src0, ctx->f16, "");
      if (ctx->options->chip_class >= VI) {
@@ -2052,11 +2052,11 @@ static LLVMValueRef 
radv_lower_gather4_integer(struct ac_llvm_context *ctx,
                             const nir_tex_instr *instr)
  {
      enum glsl_base_type stype = 
glsl_get_sampler_result_type(instr->texture->var->type);
      LLVMValueRef coord = args->addr;
      LLVMValueRef half_texel[2];
-    LLVMValueRef compare_cube_wa;
+    LLVMValueRef compare_cube_wa = NULL;
      LLVMValueRef result;
      int c;
      unsigned coord_vgpr_index = (unsigned)args->offset + 
(unsigned)args->compare;
      //TODO Rect
@@ -2778,11 +2778,12 @@ load_tcs_input(struct nir_to_llvm_context *ctx,
  static LLVMValueRef
  load_tcs_output(struct nir_to_llvm_context *ctx,
             nir_intrinsic_instr *instr)
  {
-    LLVMValueRef dw_addr, stride;
+    LLVMValueRef dw_addr;
+    LLVMValueRef stride = NULL;
      LLVMValueRef value[4], result;
      LLVMValueRef vertex_index = NULL;
      LLVMValueRef indir_index = NULL;
      unsigned const_index = 0;
      unsigned param;
@@ -2817,11 +2818,12 @@ static void
  store_tcs_output(struct nir_to_llvm_context *ctx,
           nir_intrinsic_instr *instr,
           LLVMValueRef src,
           unsigned writemask)
  {
-    LLVMValueRef stride, dw_addr;
+    LLVMValueRef dw_addr;
+    LLVMValueRef stride = NULL;
      LLVMValueRef buf_addr = NULL;
      LLVMValueRef vertex_index = NULL;
      LLVMValueRef indir_index = NULL;
      unsigned const_index = 0;
      unsigned param;
@@ -3817,12 +3819,13 @@ static LLVMValueRef visit_interp(struct 
nir_to_llvm_context *ctx,
  {
      LLVMValueRef result[2];
      LLVMValueRef interp_param, attr_number;
      unsigned location;
      unsigned chan;
-    LLVMValueRef src_c0, src_c1;
-    LLVMValueRef src0;
+    LLVMValueRef src_c0 = NULL;
+    LLVMValueRef src_c1 = NULL;
+    LLVMValueRef src0 = NULL;

I think this one actually involves a real bug. Specifically, I suspect 
that the

     if (location == INTERP_SAMPLE || location == INTERP_CENTER) {

below should really be

     if (location == INTERP_SAMPLE || location == INTERP_OFFSET) {

because src_c0/c1 are set, and the enclosed logic makes sense, only in 
the nir_intrinsic_interp_var_at_{sample,offset} cases.

Never mind, that part is misleading. location refers to the base 
location, not the final location of the sample, and it can never be 
INTERP_SAMPLE.

It'd make sense to to change it to

    if (location == INTERP_CENTER) [

Whether or not you decide to include that in the patch, it's

Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>



Apart from that, the patch looks good to me.

Cheers,
Nicolai


      int input_index = instr->variables[0]->var->data.location - 
VARYING_SLOT_VAR0;
      switch (instr->intrinsic) {
      case nir_intrinsic_interp_var_at_centroid:
          location = INTERP_CENTROID;
          break;





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev