[Mesa-dev] [PATCH] intel/fs: Prevent emission of IR instructions not aligned to their own execution size.

[Sagar Ghuge]

Tested-by: Sagar Ghuge <sagar.ghuge at intel.com>


On 11/8/18 5:26 PM, Francisco Jerez wrote:
> This can occur during payload setup of SIMD-split send message
> instructions, which can lead to the emission of header setup
> instructions with a non-zero channel group and fixed SIMD width.  Such
> instructions could end up using undefined channel enable signals
> except they don't care since they're always marked force_writemask_all.
> 
> Not known to affect correctness of any workload at this point, but it
> would be trivial to back-port to stable if something comes up.
> 
> Reported-by: Sagar Ghuge <sagar.ghuge at intel.com>
> ---
>  src/intel/compiler/brw_fs_builder.h | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs_builder.h b/src/intel/compiler/brw_fs_builder.h
> index 0cafaf50e56..4846820722c 100644
> --- a/src/intel/compiler/brw_fs_builder.h
> +++ b/src/intel/compiler/brw_fs_builder.h
> @@ -114,11 +114,25 @@ namespace brw {
>        fs_builder
>        group(unsigned n, unsigned i) const
>        {
> -         assert(force_writemask_all ||
> -                (n <= dispatch_width() && i < dispatch_width() / n));
>           fs_builder bld = *this;
> +
> +         if (n <= dispatch_width() && i < dispatch_width() / n) {
> +            bld._group += i * n;
> +         } else {
> +            /* The requested channel group isn't a subset of the channel group
> +             * of this builder, which means that the resulting instructions
> +             * would use (potentially undefined) channel enable signals not
> +             * specified by the parent builder.  That's only valid if the
> +             * instruction doesn't have per-channel semantics, in which case
> +             * we should clear off the default group index in order to prevent
> +             * emitting instructions with channel group not aligned to their
> +             * own execution size.
> +             */
> +            assert(force_writemask_all);
> +            bld._group = 0;
> +         }
> +
>           bld._dispatch_width = n;
> -         bld._group += i * n;
>           return bld;
>        }
>  
>