From 71392fff2527c96523f7a4ccd14c9fe3bcd3d56d Mon Sep 17 00:00:00 2001 From: Friedrich Vock Date: Sun, 15 Dec 2024 17:15:30 +0100 Subject: [PATCH] aco: Fix dead instruction/index handling for try_insert_saveexec_out_of_loop The loop checking if exec is overwritten didn't check for NULL instructions, and didn't fix up reg write indices after inserting instructions. Fixes: fcd94a8c ("aco: move try_optimize_branching_sequence() to postRA optimizations") Part-of: --- src/amd/compiler/aco_optimizer_postRA.cpp | 29 ++++++++++++++++------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/amd/compiler/aco_optimizer_postRA.cpp b/src/amd/compiler/aco_optimizer_postRA.cpp index c4209f65a91..67e8b71a09e 100644 --- a/src/amd/compiler/aco_optimizer_postRA.cpp +++ b/src/amd/compiler/aco_optimizer_postRA.cpp @@ -867,6 +867,8 @@ try_insert_saveexec_out_of_loop(pr_opt_ctx& ctx, Block* block, Definition saved_ /* Check if exec is written, or the copy's dst overwritten in the loop header. */ for (unsigned i = 0; i < saveexec_pos; i++) { + if (!block->instructions[i]) + continue; if (block->instructions[i]->writes_exec()) return false; if (instr_overwrites(block->instructions[i].get(), saved_exec.physReg(), saved_exec.size())) @@ -906,6 +908,19 @@ try_insert_saveexec_out_of_loop(pr_opt_ctx& ctx, Block* block, Definition saved_ return false; } +void +fixup_reg_writes(pr_opt_ctx& ctx, unsigned start) +{ + const unsigned current_idx = ctx.current_instr_idx; + for (unsigned i = start; i < current_idx; i++) { + ctx.current_instr_idx = i; + if (ctx.current_block->instructions[i]) + save_reg_writes(ctx, ctx.current_block->instructions[i]); + } + + ctx.current_instr_idx = current_idx; +} + bool try_optimize_branching_sequence(pr_opt_ctx& ctx, aco_ptr& exec_copy) { @@ -1145,8 +1160,11 @@ try_optimize_branching_sequence(pr_opt_ctx& ctx, aco_ptr& exec_copy */ if (ctx.current_block->kind & block_kind_loop_header) { if (try_insert_saveexec_out_of_loop(ctx, ctx.current_block, exec_copy_def, - exec_val_idx.instr)) + exec_val_idx.instr)) { + /* We inserted something after the last phi, so fixup indices from the start. */ + fixup_reg_writes(ctx, 0); return true; + } } Instruction* copy = create_instruction(aco_opcode::p_parallelcopy, Format::PSEUDO, 1, 1); copy->definitions[0] = exec_copy_def; @@ -1155,14 +1173,7 @@ try_optimize_branching_sequence(pr_opt_ctx& ctx, aco_ptr& exec_copy ctx.current_block->instructions.emplace(it, copy); /* Fixup indices after inserting an instruction. */ - const unsigned current_idx = ctx.current_instr_idx; - for (unsigned i = exec_val_idx.instr; i < current_idx; i++) { - ctx.current_instr_idx = i; - if (ctx.current_block->instructions[i]) - save_reg_writes(ctx, ctx.current_block->instructions[i]); - } - - ctx.current_instr_idx = current_idx; + fixup_reg_writes(ctx, exec_val_idx.instr); return true; }