From 8c87082c940ddba90e7bd0aefdf834eb2c335a74 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Fri, 15 May 2020 14:57:40 -0500 Subject: [PATCH] nir: Validate jump instructions as an instruction type This has the downside of putting block successor validation in two places that are a bit further apart. However, handling them as a special case makes the code more confusing than needed. At least two different people have not noticed that we don't have jump instruction validation in the last week or two and added it. Being able to search for validate_jump_instr is useful. Reviewed-by: Caio Marcelo de Oliveira Filho Reviewed-by: Karol Herbst Reviewed-by: Alyssa Rosenzweig Part-of: --- src/compiler/nir/nir_validate.c | 69 +++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c index 998017d504a..e9fc6150c24 100644 --- a/src/compiler/nir/nir_validate.c +++ b/src/compiler/nir/nir_validate.c @@ -749,6 +749,43 @@ validate_phi_instr(nir_phi_instr *instr, validate_state *state) state->block->predecessors->entries); } +static void +validate_jump_instr(nir_jump_instr *instr, validate_state *state) +{ + nir_block *block = state->block; + validate_assert(state, &instr->instr == nir_block_last_instr(block)); + + switch (instr->type) { + case nir_jump_return: + validate_assert(state, block->successors[0] == state->impl->end_block); + validate_assert(state, block->successors[1] == NULL); + break; + + case nir_jump_break: + validate_assert(state, state->loop != NULL); + if (state->loop) { + nir_block *after = + nir_cf_node_as_block(nir_cf_node_next(&state->loop->cf_node)); + validate_assert(state, block->successors[0] == after); + } + validate_assert(state, block->successors[1] == NULL); + break; + + case nir_jump_continue: + validate_assert(state, state->loop != NULL); + if (state->loop) { + nir_block *first = nir_loop_first_block(state->loop); + validate_assert(state, block->successors[0] == first); + } + validate_assert(state, block->successors[1] == NULL); + break; + + default: + validate_assert(state, !"Invalid jump instruction type"); + break; + } +} + static void validate_instr(nir_instr *instr, validate_state *state) { @@ -790,6 +827,7 @@ validate_instr(nir_instr *instr, validate_state *state) break; case nir_instr_type_jump: + validate_jump_instr(nir_instr_as_jump(instr), state); break; default: @@ -848,10 +886,6 @@ validate_block(nir_block *block, validate_state *state) nir_instr_prev(instr)->type == nir_instr_type_phi); } - if (instr->type == nir_instr_type_jump) { - validate_assert(state, instr == nir_block_last_instr(block)); - } - validate_instr(instr, state); } @@ -874,32 +908,7 @@ validate_block(nir_block *block, validate_state *state) pred->successors[1] == block); } - if (!exec_list_is_empty(&block->instr_list) && - nir_block_last_instr(block)->type == nir_instr_type_jump) { - validate_assert(state, block->successors[1] == NULL); - nir_jump_instr *jump = nir_instr_as_jump(nir_block_last_instr(block)); - switch (jump->type) { - case nir_jump_break: { - nir_block *after = - nir_cf_node_as_block(nir_cf_node_next(&state->loop->cf_node)); - validate_assert(state, block->successors[0] == after); - break; - } - - case nir_jump_continue: { - nir_block *first = nir_loop_first_block(state->loop); - validate_assert(state, block->successors[0] == first); - break; - } - - case nir_jump_return: - validate_assert(state, block->successors[0] == state->impl->end_block); - break; - - default: - unreachable("bad jump type"); - } - } else { + if (!nir_block_ends_in_jump(block)) { nir_cf_node *next = nir_cf_node_next(&block->cf_node); if (next == NULL) { switch (state->parent_node->type) { -- 2.30.2