Module: Mesa Branch: main Commit: 02f9bbf6f351accb1d52d8c0eddae636efab38ae URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=02f9bbf6f351accb1d52d8c0eddae636efab38ae
Author: Ian Romanick <ian.d.roman...@intel.com> Date: Mon Sep 11 09:10:47 2023 -0700 intel/compiler: Add basic CFG validation v2: Use _mesa_shader_stage_to_abbrev(stage) instead of stage_abbrev. Noticed by Caio and GCC. That's what I get for not recompiling after rebasing. Wrap cfg_t::validate in NDEBUG magic. Suggested by Caio. Reviewed-by: Caio Oliveira <caio.olive...@intel.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25216> --- src/intel/compiler/brw_cfg.cpp | 51 ++++++++++++++++++++++++++++++++++ src/intel/compiler/brw_cfg.h | 6 ++++ src/intel/compiler/brw_fs_validate.cpp | 2 ++ src/intel/compiler/brw_vec4.cpp | 2 ++ 4 files changed, 61 insertions(+) diff --git a/src/intel/compiler/brw_cfg.cpp b/src/intel/compiler/brw_cfg.cpp index 4ee8cc82320..7245fcf651b 100644 --- a/src/intel/compiler/brw_cfg.cpp +++ b/src/intel/compiler/brw_cfg.cpp @@ -615,3 +615,54 @@ cfg_t::dump_cfg() } printf("}\n"); } + +#define cfgv_assert(assertion) \ + { \ + if (!(assertion)) { \ + fprintf(stderr, "ASSERT: CFG validation in %s failed!\n", stage_abbrev); \ + fprintf(stderr, "%s:%d: '%s' failed\n", __FILE__, __LINE__, #assertion); \ + abort(); \ + } \ + } + +#ifndef NDEBUG +void +cfg_t::validate(const char *stage_abbrev) +{ + foreach_block(block, this) { + foreach_list_typed(bblock_link, successor, link, &block->children) { + /* Each successor of a block must have a predecessor link back to + * the block. + */ + bool successor_links_back_to_predecessor = false; + bblock_t *succ_block = successor->block; + + foreach_list_typed(bblock_link, predecessor, link, &succ_block->parents) { + if (predecessor->block == block) { + successor_links_back_to_predecessor = true; + break; + } + } + + cfgv_assert(successor_links_back_to_predecessor); + } + + foreach_list_typed(bblock_link, predecessor, link, &block->parents) { + /* Each predecessor of a block must have a successor link back to + * the block. + */ + bool predecessor_links_back_to_successor = false; + bblock_t *pred_block = predecessor->block; + + foreach_list_typed(bblock_link, successor, link, &pred_block->children) { + if (successor->block == block) { + predecessor_links_back_to_successor = true; + break; + } + } + + cfgv_assert(predecessor_links_back_to_successor); + } + } +} +#endif diff --git a/src/intel/compiler/brw_cfg.h b/src/intel/compiler/brw_cfg.h index 9e2726f453b..36e82cad224 100644 --- a/src/intel/compiler/brw_cfg.h +++ b/src/intel/compiler/brw_cfg.h @@ -325,6 +325,12 @@ struct cfg_t { void dump(); void dump_cfg(); +#ifdef NDEBUG + void validate(UNUSED const char *stage_abbrev) { } +#else + void validate(const char *stage_abbrev); +#endif + /** * Propagate bblock_t::end_ip_delta data through the CFG. */ diff --git a/src/intel/compiler/brw_fs_validate.cpp b/src/intel/compiler/brw_fs_validate.cpp index c285cc2dc26..499bc8181c3 100644 --- a/src/intel/compiler/brw_fs_validate.cpp +++ b/src/intel/compiler/brw_fs_validate.cpp @@ -90,6 +90,8 @@ void fs_visitor::validate() { + cfg->validate(_mesa_shader_stage_to_abbrev(stage)); + foreach_block_and_inst (block, fs_inst, inst, cfg) { switch (inst->opcode) { case SHADER_OPCODE_SEND: diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp index 67baec9f44a..f80dfff63bf 100644 --- a/src/intel/compiler/brw_vec4.cpp +++ b/src/intel/compiler/brw_vec4.cpp @@ -2392,6 +2392,7 @@ vec4_visitor::run() emit_thread_end(); calculate_cfg(); + cfg->validate(_mesa_shader_stage_to_abbrev(stage)); /* Before any optimization, push array accesses out to scratch * space where we need them to be. This pass may allocate new @@ -2417,6 +2418,7 @@ vec4_visitor::run() backend_shader::dump_instructions(filename); \ } \ \ + cfg->validate(_mesa_shader_stage_to_abbrev(stage)); \ progress = progress || this_progress; \ this_progress; \ })