[Mesa-stable] [Mesa-dev] [PATCH 2/2] i965/fs: Register allocator shoudn't use grf127 for sends dest (v2)

[Chema Casanova]

This patch is still pending of review.

Adding also Cc: to stable as if fixes some cts issues.

Chema


El 19/04/18 a las 02:38, Jose Maria Casanova Crespo escribió:
> Since Gen8+ Intel PRM states that "r127 must not be used for return
> address when there is a src and dest overlap in send instruction."
> 
> This patch implements this restriction creating new grf127_send_hack_node
> at the register allocator. This node has a fixed assignation to grf127.
> 
> For vgrf that are used as destination of send messages we create node
> interfereces with the grf127_send_hack_node. So the register allocator
> will never assign to these vgrf a register that involves grf127.
> 
> If dispatch_width > 8 we don't create these interferences to the because
> all instructions have node interferences between sources and destination.
> That is enough to avoid the r127 restriction.
> 
> This fixes CTS tests that raised this issue as they were executed as SIMD8:
>   dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.uniform_16struct_to_32struct.uniform_buffer_block_vert
>   dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.uniform_16struct_to_32struct.uniform_buffer_block_tessc
> 
> Shader-db results on Skylake:
>    total instructions in shared programs: 7686798 -> 7686797 (<.01%)
>    instructions in affected programs: 301 -> 300 (-0.33%)
>    helped: 1
>    HURT: 0
> 
>    total cycles in shared programs: 337092322 -> 337091919 (<.01%)
>    cycles in affected programs: 22420415 -> 22420012 (<.01%)
>    helped: 712
>    HURT: 588
> 
> Shader-db results on Broadwell:
> 
>    total instructions in shared programs: 7658574 -> 7658625 (<.01%)
>    instructions in affected programs: 19610 -> 19661 (0.26%)
>    helped: 3
>    HURT: 4
> 
>    total cycles in shared programs: 340694553 -> 340676378 (<.01%)
>    cycles in affected programs: 24724915 -> 24706740 (-0.07%)
>    helped: 998
>    HURT: 916
> 
>    total spills in shared programs: 4300 -> 4311 (0.26%)
>    spills in affected programs: 333 -> 344 (3.30%)
>    helped: 1
>    HURT: 3
> 
>    total fills in shared programs: 5370 -> 5378 (0.15%)
>    fills in affected programs: 274 -> 282 (2.92%)
>    helped: 1
>    HURT: 3
> 
> v2: Avoid duplicating register classes without grf127. Let's use a node
>     with a fixed assignation to grf127 and create interferences to send
>     message vgrf destinations. (Eric Anholt)
> ---
>  src/intel/compiler/brw_fs_reg_allocate.cpp | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp b/src/intel/compiler/brw_fs_reg_allocate.cpp
> index ec8e116cb38..59e047483c0 100644
> --- a/src/intel/compiler/brw_fs_reg_allocate.cpp
> +++ b/src/intel/compiler/brw_fs_reg_allocate.cpp
> @@ -548,6 +548,9 @@ fs_visitor::assign_regs(bool allow_spilling, bool spill_all)
>     int first_mrf_hack_node = node_count;
>     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)
> +      node_count ++;
>     struct ra_graph *g =
>        ra_alloc_interference_graph(compiler->fs_reg_sets[rsi].regs, node_count);
>  
> @@ -653,6 +656,28 @@ fs_visitor::assign_regs(bool allow_spilling, bool spill_all)
>        }
>     }
>  
> +   if (devinfo->gen >= 8 && dispatch_width == 8) {
> +      /* At Intel Broadwell PRM, vol 07, section "Instruction Set Reference",
> +       * subsection "EUISA Instructions", Send Message (page 990):
> +       *
> +       * "r127 must not be used for return address when there is a src and
> +       * dest overlap in send instruction."
> +       *
> +       * 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.
> +       */
> +      ra_set_node_reg(g, grf127_send_hack_node, 127);
> +      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);
> +         }
> +      }
> +   }
> +
>     /* Debug of register spilling: Go spill everything. */
>     if (unlikely(spill_all)) {
>        int reg = choose_spill_reg(g);
>