[Mesa-dev] [PATCH 16/24] i965/fs: Fix cmod propagation not to propagate non-identity cmod into CMP(N).

[Jason Ekstrand]

On Thu, May 26, 2016 at 8:46 PM, Francisco Jerez <currojerez at riseup.net>
wrote:

> The conditional mod of these instructions determines the semantics of
> the comparison itself (rather than being evaluated based on the result
> of the instruction as is usually the case for most other instructions
> that allow conditional mods), so it's in general not legal to
> propagate a conditional mod into a CMP instruction.  This prevents
> cmod propagation from (mis)optimizing:
>
>  cmp.z.f0 tmp, ...
>  mov.z.f0 null, tmp
>
> into:
>
>  cmp.z.f0 tmp, ...
>
> which gives the negation of the flag result of the original sequence.
> I could reproduce this easily with SIMD32 but I don't see any reason
> why the problem would be SIMD32-specific, it was most likely working
> by luck.
>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> index 98d4ff6..af1e3eb 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> @@ -129,6 +129,18 @@ opt_cmod_propagation_local(const brw_device_info
> *devinfo, bblock_t *block)
>                 break;
>              }
>
> +            /* The conditional mod of the CMP/CMPN instructions behaves
> +             * specially because the flag output is not calculated from
> the
> +             * result of the instruction, but the other way around, which
> +             * means that even if the condmod to propagate and the condmod
> +             * from the CMP instruction are the same they will in general
> give
> +             * different results because they are evaluated based on
> different
> +             * inputs.
> +             */
> +            if (scan_inst->opcode == BRW_OPCODE_CMP ||
> +                scan_inst->opcode == BRW_OPCODE_CMPN)
> +               break;
>

This should also be a problem with the vec4 version.  Mind fixing that
too?  You can do it as part of this patch or as a follow-on.  I don't care.
--Jason


> +
>              /* Otherwise, try propagating the conditional. */
>              enum brw_conditional_mod cond =
>                 inst->src[0].negate ? brw_swap_cmod(inst->conditional_mod)
> --
> 2.7.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160527/decbdc53/attachment.html>