From ed3e4c16dce6d14cc6540d221708c9e734cf251a Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Mon, 8 Apr 2024 18:10:12 -0700 Subject: [PATCH] intel/brw: Do not create empty basic blocks when removing instructions If there's only a single instruction in a basic block, then removing it would create an empty block. We seem to have trouble representing those as there are no instructions with an IP inside the block; several places mess up connections. While most blocks end in control flow instructions (which are rarely eliminated), ones preceding a DO instruction may end in an ordinary instruction. This makes such blocks tricky to merge with adjacent blocks - they may be between loops. Any optimization pass may may find such an instruction and want to eliminate it, and most of them are unprepared to perform such CFG link surgery. Nor do we want to make every pass aware of this issue. To work around this, we simply replace an instruction with a NOP when removing it from a block containing only that instruction, leaving the block in place. Reviewed-by: Ian Romanick Part-of: --- src/intel/compiler/brw_dead_control_flow.cpp | 7 +++++++ src/intel/compiler/brw_fs_dead_code_eliminate.cpp | 4 +++- src/intel/compiler/brw_fs_generator.cpp | 3 +++ src/intel/compiler/brw_shader.cpp | 8 ++++++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_dead_control_flow.cpp b/src/intel/compiler/brw_dead_control_flow.cpp index 2cdab6eafbd..0252bfdebe5 100644 --- a/src/intel/compiler/brw_dead_control_flow.cpp +++ b/src/intel/compiler/brw_dead_control_flow.cpp @@ -110,6 +110,13 @@ brw_fs_opt_dead_control_flow_eliminate(fs_visitor &s) if_inst->predicate_inverse = !if_inst->predicate_inverse; else_inst->remove(else_block); + progress = true; + } else if (inst->opcode == BRW_OPCODE_NOP && + prev_block->can_combine_with(block) && + exec_list_is_singular(&block->parents) && + exec_list_is_singular(&prev_block->children)) { + prev_block->combine_with(block); + inst->remove(prev_block); progress = true; } } diff --git a/src/intel/compiler/brw_fs_dead_code_eliminate.cpp b/src/intel/compiler/brw_fs_dead_code_eliminate.cpp index 13e81b9e3d4..53db51bfbc2 100644 --- a/src/intel/compiler/brw_fs_dead_code_eliminate.cpp +++ b/src/intel/compiler/brw_fs_dead_code_eliminate.cpp @@ -106,7 +106,9 @@ brw_fs_opt_dead_code_eliminate(fs_visitor &s) } } - if (inst->dst.is_null() && can_eliminate(devinfo, inst, flag_live)) { + if (inst->dst.is_null() && can_eliminate(devinfo, inst, flag_live) && + !(inst->opcode == BRW_OPCODE_NOP && + exec_list_is_singular(&block->instructions))) { inst->opcode = BRW_OPCODE_NOP; progress = true; } diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp index 22471d10a09..f7209f49e42 100644 --- a/src/intel/compiler/brw_fs_generator.cpp +++ b/src/intel/compiler/brw_fs_generator.cpp @@ -901,6 +901,9 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width, assert(inst->mlen <= BRW_MAX_MSG_LENGTH * reg_unit(devinfo)); switch (inst->opcode) { + case BRW_OPCODE_NOP: + brw_NOP(p); + break; case BRW_OPCODE_SYNC: assert(src[0].file == BRW_IMMEDIATE_VALUE); brw_SYNC(p, tgl_sync_function(src[0].ud)); diff --git a/src/intel/compiler/brw_shader.cpp b/src/intel/compiler/brw_shader.cpp index fb4fd3b4d42..dba0a9bccfa 100644 --- a/src/intel/compiler/brw_shader.cpp +++ b/src/intel/compiler/brw_shader.cpp @@ -577,6 +577,14 @@ fs_inst::remove(bblock_t *block, bool defer_later_block_ip_updates) { assert(inst_is_in_block(block, this) || !"Instruction not in block"); + if (exec_list_is_singular(&block->instructions)) { + this->opcode = BRW_OPCODE_NOP; + this->resize_sources(0); + this->dst = fs_reg(); + this->size_written = 0; + return; + } + if (defer_later_block_ip_updates) { block->end_ip_delta--; } else {