[Mesa-dev] [PATCH 19/22] i965/vec4: fix SIMD-with lowering for CMP/MOV instructions with conditional modifiers

[Matt Turner]

On Thu, Jan 5, 2017 at 5:07 AM, Samuel Iglesias Gonsálvez
<siglesias at igalia.com> wrote:
> From: "Juan A. Suarez Romero" <jasuarez at igalia.com>
>
> When splitting a CMP/MOV instruction with NULL dest, DF sources, and
> conditional modifier; we can't use directly the flag registers, as they will
> have the wrong results in IVB/VLV after the scalarization.
>
> Rather, we need to store the result in a temporary register, and then use
> that register to set proper the flags values.
>
> If a MOV has a null destination register and a conditional modifier, it
> can be replaced with a CMP against zero with the same conditional
> modifier. By doing this replacement, we can do the SIMD lowering
> without any problem.
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 80 +++++++++++++++++++++++++++++++---
>  1 file changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index c654c8b..74ab866 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -2190,6 +2190,46 @@ vec4_visitor::lower_simd_width()
>         * value of the instruction's dst.
>         */
>        bool needs_temp = dst_src_regions_overlap(inst);
> +
> +      /* When splitting instructions with conditional modifiers and NULL
> +       * dest we can't rely directly on the flags to store the result. Rather,
> +       * we need first to enqueue the result in a temporary register, and then
> +       * move those values into flags.
> +       */
> +      bool inst_df_dst_null =
> +         inst->dst.is_null() && inst->exec_data_size() == 8 &&
> +         inst->conditional_mod != BRW_CONDITIONAL_NONE;
> +
> +      if (inst_df_dst_null) {
> +         /* If there are other DF instructions with NULL destination,
> +          * we need to verify if we can use the temporary register or
> +          * if we need an extra lowering step.
> +          */
> +         assert(inst->opcode == BRW_OPCODE_MOV ||
> +                inst->opcode == BRW_OPCODE_CMP);
> +
> +         /* Replace MOV.XX with null destination with the equivalente CMP.XX

Typo: extra 'e' on equivalent

Otherwise looks fine to me. I do hope that Curro has a chance to look
over the series.