panfrost: Fix two uninitialized accesses in compiler

Submitted by Tomeu Vizoso on May 7, 2019, 3:33 p.m.

Details

Message ID 20190507153312.22181-1-tomeu.vizoso@collabora.com
State New
Headers show
Series "panfrost: Fix two uninitialized accesses in compiler" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Tomeu Vizoso May 7, 2019, 3:33 p.m.
Valgrind was complaining of those.

NIR_PASS only sets progress to TRUE if there was progress.

nir_const_load_to_arr() only sets as many constants as components has
the instruction.

This was causing some dEQP tests to flip-flop.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 src/gallium/drivers/panfrost/midgard/midgard_compile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 3a1f805702e2..0cdde46028fc 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -915,7 +915,7 @@  optimise_nir(nir_shader *nir)
                 NIR_PASS(progress, nir, nir_opt_constant_folding);
 
                 if (lower_flrp != 0) {
-                        bool lower_flrp_progress;
+                        bool lower_flrp_progress = false;
                         NIR_PASS(lower_flrp_progress,
                                  nir,
                                  nir_lower_flrp,
@@ -1020,7 +1020,7 @@  emit_load_const(compiler_context *ctx, nir_load_const_instr *instr)
 {
         nir_ssa_def def = instr->def;
 
-        float *v = ralloc_array(NULL, float, 4);
+        float *v = rzalloc_array(NULL, float, 4);
         nir_const_load_to_arr(v, instr, f32);
         _mesa_hash_table_u64_insert(ctx->ssa_constants, def.index + 1, v);
 }

Comments

Tentative R-b, but I'm baffled what the flip-flops would be about. Could
you link the list of failures introduced (we're maybe relying on buggy
behaviour anyway)?
On Tue, 7 May 2019 at 23:47, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> Tentative R-b, but I'm baffled what the flip-flops would be about. Could
> you link the list of failures introduced (we're maybe relying on buggy
> behaviour anyway)?

I was testing with
dEQP-GLES2.functional.fragment_ops.blend.equation_src_func_dst_func.add_src_color_constant_color
(should get into the habit of mentioning test case names in the commit
messages).

I guess it affected other tests, but it's hard to tell because we have
other memory issues detected by valgrind that muddy the waters.

Cheers,

Tomeu

> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Oh, I have a hunch what could be happening. I'll take a look before
merging :)