From: Jason Ekstrand <jason@jlekstrand.net>
Date: Wed, 8 Jul 2020 16:56:32 +0000 (-0500)
Subject: nir/validate: Don't abort() until after the shader has printed
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=29cba3b6954dc26ada9b5797be289f9617728974;p=mesa.git

nir/validate: Don't abort() until after the shader has printed

In the case where SSA use/def chains are broken, NIR prints out a very
cryptic error and then aborts.  This abort happens during validation
rather than after the print is complete, hiding any other errors that
may have been found.  One might think, "So what?  Fix your use/def issue
first."  However, what makes this especially bad is that, when use/def
chains are broken, there's usually a much nicer error inline in the
shader that would have been printed had we not aborted early so the
current behavior simply ensures you get the most cryptic error possible
in an already difficult-to-debug case.

While we're at it, we remove the one other case of abort() which is in
the validation of phi instruction sources.

Reviewed-by: Rob Clark <robclark@freedesktop.org>
Tested-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Reviewed-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5809>
---

diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
index ad76b94c9a9..68396d1108c 100644
--- a/src/compiler/nir/nir_validate.c
+++ b/src/compiler/nir/nir_validate.c
@@ -873,8 +873,8 @@ validate_phi_src(nir_phi_instr *instr, nir_block *pred, validate_state *state)
          return;
       }
    }
-
-   abort();
+   validate_assert(state, !"Phi does not have a source corresponding to one "
+                           "of its predecessor blocks");
 }
 
 static void
@@ -1093,42 +1093,21 @@ postvalidate_reg_decl(nir_register *reg, validate_state *state)
       validate_assert(state, entry);
       _mesa_set_remove(reg_state->uses, entry);
    }
-
-   if (reg_state->uses->entries != 0) {
-      printf("extra entries in register uses:\n");
-      set_foreach(reg_state->uses, entry)
-         printf("%p\n", entry->key);
-
-      abort();
-   }
+   validate_assert(state, reg_state->uses->entries == 0);
 
    nir_foreach_if_use(src, reg) {
       struct set_entry *entry = _mesa_set_search(reg_state->if_uses, src);
       validate_assert(state, entry);
       _mesa_set_remove(reg_state->if_uses, entry);
    }
-
-   if (reg_state->if_uses->entries != 0) {
-      printf("extra entries in register if_uses:\n");
-      set_foreach(reg_state->if_uses, entry)
-         printf("%p\n", entry->key);
-
-      abort();
-   }
+   validate_assert(state, reg_state->if_uses->entries == 0);
 
    nir_foreach_def(src, reg) {
       struct set_entry *entry = _mesa_set_search(reg_state->defs, src);
       validate_assert(state, entry);
       _mesa_set_remove(reg_state->defs, entry);
    }
-
-   if (reg_state->defs->entries != 0) {
-      printf("extra entries in register defs:\n");
-      set_foreach(reg_state->defs, entry)
-         printf("%p\n", entry->key);
-
-      abort();
-   }
+   validate_assert(state, reg_state->defs->entries == 0);
 }
 
 static void
@@ -1225,13 +1204,8 @@ validate_function_impl(nir_function_impl *impl, validate_state *state)
       postvalidate_reg_decl(reg, state);
    }
 
-   if (state->ssa_srcs->entries != 0) {
-      printf("extra dangling SSA sources:\n");
-      set_foreach(state->ssa_srcs, entry)
-         printf("%p\n", entry->key);
-
-      abort();
-   }
+   validate_assert(state, state->ssa_srcs->entries == 0);
+   _mesa_set_clear(state->ssa_srcs, NULL);
 }
 
 static void