[Mesa-dev] [RFC] i965/fs: Generalize grf127 hack to dispatch_width >

[Jason Ekstrand]

On July 10, 2018 18:50:51 Caio Marcelo de Oliveira Filho 
<caio.oliveira at intel.com> wrote:

> Change the hack to always apply, adjusting the register number
> according to the dispatch_width.
>
> The original change assumed that given for dispatch_width > 8 we
> already prevent the overlap of source and destination for send, it
> would not be necessary to explicitly add an interference with a
> register that covers r127.
>
> The problem is that the code for spilling registers ends up generating
> scratch reads, that in Gen7+ will reuse the destination register,
> causing a send with both source and destination overlaping. So prevent
> r127 (or the overlapping wider register) to be used as destination for
> sends.
>
> This patch fixes piglit test
> tests/spec/arb_compute_shader/linker/bug-93840.shader_test.
>
> Fixes: 232ed898021 "i965/fs: Register allocator shoudn't use grf127 for 
> sends dest"
> ---
>
> After more digging on the piglit failure, I came up with this
> patch. I'm still seeing crashes with for some shader-db executions
> (master have them too), but didn't have time today to drill into them.
>
> src/intel/compiler/brw_fs_reg_allocate.cpp | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp 
> b/src/intel/compiler/brw_fs_reg_allocate.cpp
> index 59e047483c0..417ddeba09c 100644
> --- a/src/intel/compiler/brw_fs_reg_allocate.cpp
> +++ b/src/intel/compiler/brw_fs_reg_allocate.cpp
> @@ -549,7 +549,7 @@ fs_visitor::assign_regs(bool allow_spilling, bool 
> spill_all)
>    if (devinfo->gen >= 7)
>       node_count += BRW_MAX_GRF - GEN7_MRF_HACK_START;
>    int grf127_send_hack_node = node_count;
> -   if (devinfo->gen >= 8 && dispatch_width == 8)
> +   if (devinfo->gen >= 8 && dispatch_width >= 8)

dispatch_width is always >= 8.

>       node_count ++;
>    struct ra_graph *g =
>       ra_alloc_interference_graph(compiler->fs_reg_sets[rsi].regs, node_count);
> @@ -656,7 +656,7 @@ fs_visitor::assign_regs(bool allow_spilling, bool 
> spill_all)
>       }
>    }
>
> -   if (devinfo->gen >= 8 && dispatch_width == 8) {
> +   if (devinfo->gen >= 8 && dispatch_width >= 8) {

Same here

>       /* At Intel Broadwell PRM, vol 07, section "Instruction Set Reference",
>        * subsection "EUISA Instructions", Send Message (page 990):
>        *
> @@ -665,12 +665,9 @@ fs_visitor::assign_regs(bool allow_spilling, bool 
> spill_all)
>        *
>        * We are avoiding using grf127 as part of the destination of send
>        * messages adding a node interference to the grf127_send_hack_node.
> -       * This node has a fixed asignment to grf127.
> -       *
> -       * We don't apply it to SIMD16 because previous code avoids any register
> -       * overlap between sources and destination.
> +       * This node has a fixed assignment that overlaps with grf127.
>        */
> -      ra_set_node_reg(g, grf127_send_hack_node, 127);
> +      ra_set_node_reg(g, grf127_send_hack_node, 128 - reg_width);
>       foreach_block_and_inst(block, fs_inst, inst, cfg) {
>          if (inst->is_send_from_grf() && inst->dst.file == VGRF) {
>             ra_add_node_interference(g, inst->dst.nr, grf127_send_hack_node);
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev