[Mesa-stable] [PATCH] intel/compiler: do not copy-propagate strided regions to ddx/ddy arguments

Juan A. Suarez Romero jasuarez at igalia.com
Mon Dec 17 10:42:24 UTC 2018


On Tue, 2018-12-11 at 12:22 +0100, Iago Toral Quiroga wrote:
> The implementation of these opcodes in the generator assumes that their
> arguments are packed, and it generates register regions based on that
> assumption.
> 
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


This patch landed in master without CCed to stable.

I understand this was done explicitly, so I'll keep this out of 18.2 stable
queue.

If this was a mistake, and you think this patch should be in 18.2 stable, let me
know.

Thanks.

	J.A.

> ---
> Although this patch hasn't landed in master yet (it is part of a larger series
> that is still in review), Jason suggested to backport it to stable. I guess it
> might be a good idea to land it early in master too if we are doing this
> though.
> 
>  .../compiler/brw_fs_copy_propagation.cpp      | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp
> index ab34b63748..a8ec1c3463 100644
> --- a/src/intel/compiler/brw_fs_copy_propagation.cpp
> +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp
> @@ -361,6 +361,20 @@ can_take_stride(fs_inst *inst, unsigned arg, unsigned stride,
>     return true;
>  }
>  
> +static bool
> +instruction_requires_packed_data(fs_inst *inst)
> +{
> +   switch (inst->opcode) {
> +   case FS_OPCODE_DDX_FINE:
> +   case FS_OPCODE_DDX_COARSE:
> +   case FS_OPCODE_DDY_FINE:
> +   case FS_OPCODE_DDY_COARSE:
> +      return true;
> +   default:
> +      return false;
> +   }
> +}
> +
>  bool
>  fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
>  {
> @@ -407,6 +421,13 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
>         inst->opcode == SHADER_OPCODE_GEN4_SCRATCH_WRITE)
>        return false;
>  
> +   /* Some instructions implemented in the generator backend, such as
> +    * derivatives, assume that their operands are packed so we can't
> +    * generally propagate strided regions to them.
> +    */
> +   if (instruction_requires_packed_data(inst) && entry->src.stride > 1)
> +      return false;
> +
>     /* Bail if the result of composing both strides would exceed the
>      * hardware limit.
>      */



More information about the mesa-stable mailing list