From 29cba3b6954dc26ada9b5797be289f9617728974 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Wed, 8 Jul 2020 11:56:32 -0500 Subject: [PATCH] nir/validate: Don't abort() until after the shader has printed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Tested-by: Marcin Ślusarz Reviewed-by: Marcin Ślusarz Reviewed-by: Alyssa Rosenzweig Part-of: --- src/compiler/nir/nir_validate.c | 40 ++++++--------------------------- 1 file changed, 7 insertions(+), 33 deletions(-) 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 -- 2.30.2