[Mesa-dev] [PATCH 3/5] i965/vec4: Fix broken saturate mask check in copy propagation.

Abdiel Janulgue abdiel.janulgue at linux.intel.com
Mon Mar 23 00:41:34 PDT 2015



On 03/20/2015 04:16 PM, Francisco Jerez wrote:
> try_copy_propagate() was checking the bit of the saturate mask for the
> arg-th component of the source to decide whether the whole source
> should be saturated (WTF?).  We need to swizzle the original saturate
> mask and check that for all enabled channels the saturate flag is
> either set or unset, as we cannot saturate a subset of destination
> components only.

This is likely the cause for that garbage rendering in Unigine Valley 
bug. Good catch!

> ---
> This change depends on PATCH 01 of my swizzle clean-up series:
> http://lists.freedesktop.org/archives/mesa-dev/2015-March/080003.html
>
>   src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> index 653700e..9437e79 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -331,10 +331,17 @@ try_copy_propagate(struct brw_context *brw, vec4_instruction *inst,
>      if (value.equals(inst->src[arg]))
>         return false;
>
> -   /* Limit saturate propagation only to SEL with src1 bounded within 1.0 and 1.0
> -    * otherwise, skip copy propagate altogether
> -    */
> -   if (entry->saturatemask & (1 << arg)) {
> +   const unsigned dst_saturate_mask = inst->dst.writemask &
> +      brw_apply_swizzle_to_mask(inst->src[arg].swizzle, entry->saturatemask);
> +
> +   if (dst_saturate_mask) {
> +      /* We either saturate all or nothing. */
> +      if (dst_saturate_mask != inst->dst.writemask)
> +         return false;
> +
> +      /* Limit saturate propagation only to SEL with src1 bounded within 1.0
> +       * and 1.0 otherwise, skip copy propagate altogether
> +       */
>         switch(inst->opcode) {
>         case BRW_OPCODE_SEL:
>            if (inst->src[1].file != IMM ||
>


More information about the mesa-dev mailing list