From 2a4efe21c5e6dba353fc67c9ea58a52a913bb970 Mon Sep 17 00:00:00 2001 From: Sviatoslav Peleshko Date: Sat, 19 Oct 2024 12:53:21 +0300 Subject: [PATCH] intel/brw/gfx9: Implement WaClearArfDependenciesBeforeEot Cc: mesa-stable Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/11928 Signed-off-by: Sviatoslav Peleshko Reviewed-by: Lionel Landwerlin Part-of: --- src/intel/compiler/brw_compile_bs.cpp | 2 + src/intel/compiler/brw_compile_cs.cpp | 2 + src/intel/compiler/brw_compile_fs.cpp | 2 + src/intel/compiler/brw_compile_gs.cpp | 2 + src/intel/compiler/brw_compile_mesh.cpp | 2 + src/intel/compiler/brw_compile_tcs.cpp | 2 + src/intel/compiler/brw_compile_tes.cpp | 2 + src/intel/compiler/brw_compile_vs.cpp | 2 + src/intel/compiler/brw_fs.h | 1 + src/intel/compiler/brw_fs_workaround.cpp | 88 ++++++++++++++++++++++++ 10 files changed, 105 insertions(+) diff --git a/src/intel/compiler/brw_compile_bs.cpp b/src/intel/compiler/brw_compile_bs.cpp index 85e0acb30e0..ef2e11c4613 100644 --- a/src/intel/compiler/brw_compile_bs.cpp +++ b/src/intel/compiler/brw_compile_bs.cpp @@ -55,6 +55,8 @@ run_bs(fs_visitor &s, bool allow_spilling) brw_allocate_registers(s, allow_spilling); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_compile_cs.cpp b/src/intel/compiler/brw_compile_cs.cpp index a7ba8cd2e4e..bba1f648e3c 100644 --- a/src/intel/compiler/brw_compile_cs.cpp +++ b/src/intel/compiler/brw_compile_cs.cpp @@ -93,6 +93,8 @@ run_cs(fs_visitor &s, bool allow_spilling) brw_allocate_registers(s, allow_spilling); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_compile_fs.cpp b/src/intel/compiler/brw_compile_fs.cpp index 7c60b9b20e6..38ff2e43698 100644 --- a/src/intel/compiler/brw_compile_fs.cpp +++ b/src/intel/compiler/brw_compile_fs.cpp @@ -1507,6 +1507,8 @@ run_fs(fs_visitor &s, bool allow_spilling, bool do_rep_send) brw_fs_workaround_emit_dummy_mov_instruction(s); brw_allocate_registers(s, allow_spilling); + + brw_fs_workaround_source_arf_before_eot(s); } return !s.failed; diff --git a/src/intel/compiler/brw_compile_gs.cpp b/src/intel/compiler/brw_compile_gs.cpp index f36974158ff..6071b5081ab 100644 --- a/src/intel/compiler/brw_compile_gs.cpp +++ b/src/intel/compiler/brw_compile_gs.cpp @@ -129,6 +129,8 @@ run_gs(fs_visitor &s) brw_allocate_registers(s, true /* allow_spilling */); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_compile_mesh.cpp b/src/intel/compiler/brw_compile_mesh.cpp index d25eebef996..84c46255f9d 100644 --- a/src/intel/compiler/brw_compile_mesh.cpp +++ b/src/intel/compiler/brw_compile_mesh.cpp @@ -323,6 +323,8 @@ run_task_mesh(fs_visitor &s, bool allow_spilling) brw_allocate_registers(s, allow_spilling); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_compile_tcs.cpp b/src/intel/compiler/brw_compile_tcs.cpp index 92950bc3ee8..5c813ef9d70 100644 --- a/src/intel/compiler/brw_compile_tcs.cpp +++ b/src/intel/compiler/brw_compile_tcs.cpp @@ -177,6 +177,8 @@ run_tcs(fs_visitor &s) brw_allocate_registers(s, true /* allow_spilling */); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_compile_tes.cpp b/src/intel/compiler/brw_compile_tes.cpp index f2b0639ade9..12aa61aab4d 100644 --- a/src/intel/compiler/brw_compile_tes.cpp +++ b/src/intel/compiler/brw_compile_tes.cpp @@ -53,6 +53,8 @@ run_tes(fs_visitor &s) brw_allocate_registers(s, true /* allow_spilling */); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_compile_vs.cpp b/src/intel/compiler/brw_compile_vs.cpp index 620ce8b65b3..082dd6deb12 100644 --- a/src/intel/compiler/brw_compile_vs.cpp +++ b/src/intel/compiler/brw_compile_vs.cpp @@ -56,6 +56,8 @@ run_vs(fs_visitor &s) brw_allocate_registers(s, true /* allow_spilling */); + brw_fs_workaround_source_arf_before_eot(s); + return !s.failed; } diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h index 441f6c96733..ef2884952b4 100644 --- a/src/intel/compiler/brw_fs.h +++ b/src/intel/compiler/brw_fs.h @@ -674,6 +674,7 @@ bool brw_fs_opt_zero_samples(fs_visitor &s); bool brw_fs_workaround_emit_dummy_mov_instruction(fs_visitor &s); bool brw_fs_workaround_memory_fence_before_eot(fs_visitor &s); +bool brw_fs_workaround_source_arf_before_eot(fs_visitor &s); bool brw_fs_workaround_nomask_control_flow(fs_visitor &s); /* Helpers. */ diff --git a/src/intel/compiler/brw_fs_workaround.cpp b/src/intel/compiler/brw_fs_workaround.cpp index 99b2a0e29b9..d60bcc050b2 100644 --- a/src/intel/compiler/brw_fs_workaround.cpp +++ b/src/intel/compiler/brw_fs_workaround.cpp @@ -271,3 +271,91 @@ brw_fs_workaround_nomask_control_flow(fs_visitor &s) return progress; } + +/** + * flags_read() and flags_written() return flag access with byte granularity, + * but for Flag Register PRM lists "Access Granularity: Word", so we can assume + * accessing any part of a word will clear its register dependency. + */ +static unsigned +bytes_bitmask_to_words(unsigned b) +{ + unsigned first_byte_mask = b & 0x55555555; + unsigned second_byte_mask = b & 0xaaaaaaaa; + return first_byte_mask | + (first_byte_mask << 1) | + second_byte_mask | + (second_byte_mask >> 1); +} + +/** + * WaClearArfDependenciesBeforeEot + * + * Flag register dependency not cleared after EOT, so we have to source them + * before EOT. We can do this with simple `mov(1) nullUD, f{0,1}UD` + * + * To avoid emitting MOVs when it's not needed, check if each block reads all + * the flags it sets. We might falsely determine register as unread if it'll be + * accessed inside the next blocks, but this still should be good enough. + */ +bool +brw_fs_workaround_source_arf_before_eot(fs_visitor &s) +{ + bool progress = false; + + if (s.devinfo->ver != 9) + return false; + + unsigned flags_unread = 0; + + foreach_block(block, s.cfg) { + unsigned flags_unread_in_block = 0; + + foreach_inst_in_block(fs_inst, inst, block) { + /* Instruction can read and write to the same flag, so the order is important */ + flags_unread_in_block &= ~bytes_bitmask_to_words(inst->flags_read(s.devinfo)); + flags_unread_in_block |= bytes_bitmask_to_words(inst->flags_written(s.devinfo)); + + /* HALT does not start its block even though it can leave a dependency */ + if (inst->opcode == BRW_OPCODE_HALT || + inst->opcode == SHADER_OPCODE_HALT_TARGET) { + flags_unread |= flags_unread_in_block; + flags_unread_in_block = 0; + } + } + + flags_unread |= flags_unread_in_block; + + if ((flags_unread & 0x0f) && (flags_unread & 0xf0)) + break; + } + + if (flags_unread) { + int eot_count = 0; + + foreach_block_and_inst_safe(block, fs_inst, inst, s.cfg) + { + if (!inst->eot) + continue; + + /* Currently, we always emit only one EOT per program, + * this WA should be updated if it ever changes. + */ + assert(++eot_count == 1); + + const fs_builder ibld(&s, block, inst); + const fs_builder ubld = ibld.exec_all().group(1, 0); + + if (flags_unread & 0x0f) + ubld.MOV(ubld.null_reg_ud(), retype(brw_flag_reg(0, 0), BRW_TYPE_UD)); + + if (flags_unread & 0xf0) + ubld.MOV(ubld.null_reg_ud(), retype(brw_flag_reg(1, 0), BRW_TYPE_UD)); + } + + progress = true; + s.invalidate_analysis(DEPENDENCY_INSTRUCTIONS); + } + + return progress; +}