From ec2e8bc33f59b55387ed39f0c4374ebdf6216342 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 6 Jul 2022 20:29:02 -0700 Subject: [PATCH] intel/compiler: Avoid copy propagating large registers into EOT messages EOT messages need to use g112-g127 for their sources. With the new opt_split_sends pass, we may be constructing an EOT message from two different registers, and be able to copy propagate the original values into those SENDs. This can cause problems if we copy propagate from a large register (say an RGBA value which is 4 GRFs in SIMD8 or 8 GRFs in SIMD16), in a situation where the SEND only read a subset of that (say the alpha value out of an RGBA texturing result). g112-127 can only hold 16 registers worth of data, and sometimes we can only use g112-126. So, we can't propagate if the GRFs in question are larger than 15 GRFs. Fixes a shader validation failure in Alan Wake. Thanks to Ian Romanick for catching this! shader-db on Icelake shows that only SIMD32 programs are affected, and the results are pretty negligable: total instructions in shared programs: 19615228 -> 19615269 (<.01%) instructions in affected programs: 10702 -> 10743 (0.38%) helped: 1 / HURT: 43 / largest change: +/- 2 instructions total cycles in shared programs: 852001706 -> 852001566 (<.01%) cycles in affected programs: 767098 -> 766958 (-0.02%) helped: 68 / HURT: 64 / largest change: +/- 774 cycles GAINED: 2 / LOST: 0 Fixes: 589b03d02f0 ("intel/fs: Opportunistically split SEND message payloads") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6803 Reviewed-by: Ian Romanick Part-of: --- .../compiler/brw_fs_copy_propagation.cpp | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp index fd3dfc157fa..8535b31c0ca 100644 --- a/src/intel/compiler/brw_fs_copy_propagation.cpp +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp @@ -488,11 +488,30 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry) entry->dst, entry->size_written)) return false; - /* Avoid propagating a FIXED_GRF register into an EOT instruction in order - * for any register allocation restrictions to be applied. + /* Send messages with EOT set are restricted to use g112-g127 (and we + * sometimes need g127 for other purposes), so avoid copy propagating + * anything that would make it impossible to satisfy that restriction. */ - if (entry->src.file == FIXED_GRF && inst->eot) - return false; + if (inst->eot) { + /* Avoid propagating a FIXED_GRF register, as that's already pinned. */ + if (entry->src.file == FIXED_GRF) + return false; + + /* We might be propagating from a large register, while the SEND only + * is reading a portion of it (say the .A channel in an RGBA value). + * We need to pin both split SEND sources in g112-g126/127, so only + * allow this if the registers aren't too large. + */ + if (inst->opcode == SHADER_OPCODE_SEND && entry->src.file == VGRF) { + int other_src = arg == 2 ? 3 : 2; + unsigned other_size = inst->src[other_src].file == VGRF ? + alloc.sizes[inst->src[other_src].nr] : + inst->size_read(other_src); + unsigned prop_src_size = alloc.sizes[entry->src.nr]; + if (other_size + prop_src_size > 15) + return false; + } + } /* Avoid propagating odd-numbered FIXED_GRF registers into the first source * of a LINTERP instruction on platforms where the PLN instruction has