[Mesa-dev] [PATCH] intel/compiler: fix node interference of simd16 instructions

[Iago Toral]

This is still pending review, any takers?
Thanks
Iago

On Wed, 2018-10-17 at 12:05 +0200, Iago Toral Quiroga wrote:
> SIMD16 instructions need to have additional interferences to prevent
> source / destination hazards when the source and destination
> registers
> are off by one register.
> 
> While we already have code to handle this, it was only running for
> SIMD16
> dispatches, however, we can have SIDM16 instructions in a SIMD8
> dispatch.
> An example of this are pull constant loads since commit
> b56fa830c6095,
> but there are more cases.
> 
> This fixes a number of CTS test failues found in work-in-progress
> tests that were hitting this situation for 16-wide pull constants
> in a SIMD8 program.
> ---
>  src/intel/compiler/brw_fs_reg_allocate.cpp | 36 ++++++++++--------
> ----
>  1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp
> b/src/intel/compiler/brw_fs_reg_allocate.cpp
> index 42ccb28de6..f72826bc41 100644
> --- a/src/intel/compiler/brw_fs_reg_allocate.cpp
> +++ b/src/intel/compiler/brw_fs_reg_allocate.cpp
> @@ -632,26 +632,24 @@ fs_visitor::assign_regs(bool allow_spilling,
> bool spill_all)
>        }
>     }
>  
> -   if (dispatch_width > 8) {
> -      /* In 16-wide dispatch we have an issue where a compressed
> -       * instruction is actually two instructions executed
> simultaneiously.
> -       * It's actually ok to have the source and destination
> registers be
> -       * the same.  In this case, each instruction over-writes its
> own
> -       * source and there's no problem.  The real problem here is if
> the
> -       * source and destination registers are off by one.  Then you
> can end
> -       * up in a scenario where the first instruction over-writes
> the
> -       * source of the second instruction.  Since the compiler
> doesn't know
> -       * about this level of granularity, we simply make the source
> and
> -       * destination interfere.
> -       */
> -      foreach_block_and_inst(block, fs_inst, inst, cfg) {
> -         if (inst->dst.file != VGRF)
> -            continue;
> +   /* In 16-wide instructions we have an issue where a compressed
> +    * instruction is actually two instructions executed
> simultaneiously.
> +    * It's actually ok to have the source and destination registers
> be
> +    * the same.  In this case, each instruction over-writes its own
> +    * source and there's no problem.  The real problem here is if
> the
> +    * source and destination registers are off by one.  Then you can
> end
> +    * up in a scenario where the first instruction over-writes the
> +    * source of the second instruction.  Since the compiler doesn't
> know
> +    * about this level of granularity, we simply make the source and
> +    * destination interfere.
> +    */
> +   foreach_block_and_inst(block, fs_inst, inst, cfg) {
> +      if (inst->exec_size < 16 || inst->dst.file != VGRF)
> +         continue;
>  
> -         for (int i = 0; i < inst->sources; ++i) {
> -            if (inst->src[i].file == VGRF) {
> -               ra_add_node_interference(g, inst->dst.nr, inst-
> >src[i].nr);
> -            }
> +      for (int i = 0; i < inst->sources; ++i) {
> +         if (inst->src[i].file == VGRF) {
> +            ra_add_node_interference(g, inst->dst.nr, inst-
> >src[i].nr);
>           }
>        }
>     }