From 4dbf9181cd53baad71a2dba7b3a9198c57ba1941 Mon Sep 17 00:00:00 2001 From: Caio Oliveira Date: Wed, 17 Jan 2024 17:21:57 -0800 Subject: [PATCH] intel/compiler: Fix rebuilding the CFG in fs_combine_constants When building the CFG the instructions are taken of the list in fs_visitor and added to the lists inside each block. The single "exec_node" in the instruction is used for those memberships. In the case the pass rebuilt the CFG, it had no instructions, so calculate_cfg() had nothing to work with. For now fix the bug by pulling all the instructions back to the original list. We can do better here, but punting until upcoming work on CFG itself. Issue found in an unpublished CTS test. Small reproduction in our unit tests now enabled. Fixes: 65237f8bbca ("intel/fs: Don't add MOV instructions to DO blocks in combine constants") Reviewed-by: Ian Romanick Part-of: --- src/intel/compiler/brw_fs_combine_constants.cpp | 10 ++++++++++ src/intel/compiler/test_fs_combine_constants.cpp | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_fs_combine_constants.cpp b/src/intel/compiler/brw_fs_combine_constants.cpp index 911c6b684f9..ed5176153da 100644 --- a/src/intel/compiler/brw_fs_combine_constants.cpp +++ b/src/intel/compiler/brw_fs_combine_constants.cpp @@ -1834,6 +1834,16 @@ fs_visitor::opt_combine_constants() } if (rebuild_cfg) { + /* When the CFG is initially built, the instructions are removed from + * the list of instructions stored in fs_visitor -- the same exec_node + * is used for membership in that list and in a block list. So we need + * to pull them back before rebuilding the CFG. + */ + assert(exec_list_length(&instructions) == 0); + foreach_block(block, cfg) { + exec_list_append(&instructions, &block->instructions); + } + delete cfg; cfg = NULL; calculate_cfg(); diff --git a/src/intel/compiler/test_fs_combine_constants.cpp b/src/intel/compiler/test_fs_combine_constants.cpp index 02f2cd0f038..805a78e0b50 100644 --- a/src/intel/compiler/test_fs_combine_constants.cpp +++ b/src/intel/compiler/test_fs_combine_constants.cpp @@ -98,7 +98,7 @@ TEST_F(FSCombineConstantsTest, Simple) ASSERT_EQ(bblock_end(block)->opcode, BRW_OPCODE_SEL); } -TEST_F(FSCombineConstantsTest, DISABLED_DoContainingDo) +TEST_F(FSCombineConstantsTest, DoContainingDo) { fs_builder bld = make_builder(shader);