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

[Samuel Iglesias Gonsálvez]

On Wednesday, 17 October 2018 12:05:42 (CET) 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

failures

> 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.

simultaneously

With these two changes,

Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>

Sam

> +    * 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);
>           }
>        }
>     }

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181108/bcbdee0e/attachment.sig>