Re: [SVE] PR91272

[Richard Sandiford <richard.sandiford@arm.com>]

Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index acdd90784dc..dfd33b142ed 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -10016,25 +10016,26 @@ vectorizable_condition (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
>        /* See whether another part of the vectorized code applies a loop
>  	 mask to the condition, or to its inverse.  */
>  
> +      vec_loop_masks *masks = NULL;
>        if (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
>  	{
> -	  scalar_cond_masked_key cond (cond_expr, ncopies);
> -	  if (loop_vinfo->scalar_cond_masked_set.contains (cond))
> -	    {
> -	      vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
> -	      loop_mask = vect_get_loop_mask (gsi, masks, ncopies, vectype, j);
> -	    }
> +	  if (reduction_type == EXTRACT_LAST_REDUCTION)
> +	    masks = &LOOP_VINFO_MASKS (loop_vinfo);
>  	  else
>  	    {
> -	      bool honor_nans = HONOR_NANS (TREE_TYPE (cond.op0));
> -	      cond.code = invert_tree_comparison (cond.code, honor_nans);
> +	      scalar_cond_masked_key cond (cond_expr, ncopies);
>  	      if (loop_vinfo->scalar_cond_masked_set.contains (cond))
> +		masks = &LOOP_VINFO_MASKS (loop_vinfo);
> +	      else
>  		{
> -		  vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
> -		  loop_mask = vect_get_loop_mask (gsi, masks, ncopies,
> -						  vectype, j);
> -		  cond_code = cond.code;
> -		  swap_cond_operands = true;
> +		  bool honor_nans = HONOR_NANS (TREE_TYPE (cond.op0));
> +		  cond.code = invert_tree_comparison (cond.code, honor_nans);
> +		  if (loop_vinfo->scalar_cond_masked_set.contains (cond))
> +		    {
> +		      masks = &LOOP_VINFO_MASKS (loop_vinfo);
> +		      cond_code = cond.code;
> +		      swap_cond_operands = true;
> +		    }
>  		}
>  	    }
>  	}
> @@ -10116,6 +10117,13 @@ vectorizable_condition (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
>            vec_then_clause = vec_oprnds2[i];
>            vec_else_clause = vec_oprnds3[i];
>  
> +          if (masks)
> +	    {
> +	      unsigned vec_num = vec_oprnds0.length ();
> +	      loop_mask = vect_get_loop_mask (gsi, masks, vec_num * ncopies,
> +					      vectype, vec_num * j + i);
> +	    }
> +

I don't think we need an extra "if" here.  "loop_mask" only feeds
the later "if (loop_mask)" block, so we might as well change that
later "if" to "if (masks)" and make the "loop_mask" variable local
to the "if" body.

>  	  if (swap_cond_operands)
>  	    std::swap (vec_then_clause, vec_else_clause);
>  
> @@ -10194,23 +10202,6 @@ vectorizable_condition (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
>  		  vec_compare = tmp;
>  		}
>  
> -	      tree tmp2 = make_ssa_name (vec_cmp_type);
> -	      gassign *g = gimple_build_assign (tmp2, BIT_AND_EXPR,
> -						vec_compare, loop_mask);
> -	      vect_finish_stmt_generation (stmt_info, g, gsi);
> -	      vec_compare = tmp2;
> -	    }
> -
> -	  if (reduction_type == EXTRACT_LAST_REDUCTION)
> -	    {
> -	      if (!is_gimple_val (vec_compare))
> -		{
> -		  tree vec_compare_name = make_ssa_name (vec_cmp_type);
> -		  gassign *new_stmt = gimple_build_assign (vec_compare_name,
> -							   vec_compare);
> -		  vect_finish_stmt_generation (stmt_info, new_stmt, gsi);
> -		  vec_compare = vec_compare_name;
> -		}

This form is simpler than:

	      if (COMPARISON_CLASS_P (vec_compare))
		{
		  tree tmp = make_ssa_name (vec_cmp_type);
		  tree op0 = TREE_OPERAND (vec_compare, 0);
		  tree op1 = TREE_OPERAND (vec_compare, 1);
		  gassign *g = gimple_build_assign (tmp,
						    TREE_CODE (vec_compare),
						    op0, op1);
		  vect_finish_stmt_generation (stmt_info, g, gsi);
		  vec_compare = tmp;
		}

so I think it'd be better to keep the EXTRACT_LAST_REDUCTION version.

Thanks,
Richard