diff mbox series

vect: Enhance condition check to use partial vectors in vectorizable_condition

Message ID 85dd9d0a-f823-ed03-7a19-dcc23c1789dc@linux.ibm.com
State New
Headers show
Series vect: Enhance condition check to use partial vectors in vectorizable_condition | expand

Commit Message

Kewen.Lin July 8, 2020, 7:07 a.m. UTC 
Hi,

This patch is derived from the review of vector with length patch series.
The length-based partial vector approach doesn't support reduction so far,
so we would like to disable vectorization with partial vectors explicitly
for it in vectorizable_condition.  Otherwise, it will cause some unexpected
failures for a few cases like gcc.dg/vect/pr65947-2.c.

But if we disable it for the cases excepting for reduction_type equal to
EXTRACT_LAST_REDUCTION, it cause one regression failure on aarch64:
  gcc.target/aarch64/sve/reduc_8.c -march=armv8.2-a+sve

The disabling makes the outer loop can't work with partial vectors, the
check fails.  But the case is safe to adopt it.  As Richard S. pointed out
in the review comments, the extra inactive lanes only matter for double
reductions, so this patch is to permit vectorization with partial vectors
for cases EXTRACT_LAST_REDUCTION or nested-cycle reduction.

Testing is ongoing, is it ok for trunk if the testing goes well?

BR,
Kewen
-----
gcc/ChangeLog:

	* tree-vect-stmts.c (vectorizable_condition): Prohibit vectorization
	with partial vectors explicitly excepting for EXTRACT_LAST_REDUCTION
	or nested-cycle reduction.

Comments

Richard Sandiford July 8, 2020, 12:56 p.m. UTC | #1 
"Kewen.Lin" <linkw@linux.ibm.com> writes:
> Hi,
>
> This patch is derived from the review of vector with length patch series.
> The length-based partial vector approach doesn't support reduction so far,
> so we would like to disable vectorization with partial vectors explicitly
> for it in vectorizable_condition.  Otherwise, it will cause some unexpected
> failures for a few cases like gcc.dg/vect/pr65947-2.c.
>
> But if we disable it for the cases excepting for reduction_type equal to
> EXTRACT_LAST_REDUCTION, it cause one regression failure on aarch64:
>   gcc.target/aarch64/sve/reduc_8.c -march=armv8.2-a+sve
>
> The disabling makes the outer loop can't work with partial vectors, the
> check fails.  But the case is safe to adopt it.  As Richard S. pointed out
> in the review comments, the extra inactive lanes only matter for double
> reductions, so this patch is to permit vectorization with partial vectors
> for cases EXTRACT_LAST_REDUCTION or nested-cycle reduction.
>
> Testing is ongoing, is it ok for trunk if the testing goes well?
>
> BR,
> Kewen
> -----
> gcc/ChangeLog:
>
> 	* tree-vect-stmts.c (vectorizable_condition): Prohibit vectorization
> 	with partial vectors explicitly excepting for EXTRACT_LAST_REDUCTION
> 	or nested-cycle reduction.
>
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 40e2664f93b..c23520aceab 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -9968,11 +9968,16 @@ vectorizable_condition (vec_info *vinfo,
>  	  return false;
>  	}
>  
> -      if (loop_vinfo
> -	  && LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo)
> -	  && reduction_type == EXTRACT_LAST_REDUCTION)
> -	vect_record_loop_mask (loop_vinfo, &LOOP_VINFO_MASKS (loop_vinfo),
> -			       ncopies * vec_num, vectype, NULL);
> +      if (loop_vinfo && for_reduction
> +	  && LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo))
> +	{
> +	  if (reduction_type == EXTRACT_LAST_REDUCTION)
> +	    vect_record_loop_mask (loop_vinfo, &LOOP_VINFO_MASKS (loop_vinfo),
> +				   ncopies * vec_num, vectype, NULL);
> +	  /* Extra inactive lanes should be safe for vect_nested_cycle.  */
> +	  else if (STMT_VINFO_DEF_TYPE (reduc_info) != vect_nested_cycle)
> +	    LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;

We should print a dump message when setting this to false.  E.g.:

	  if (dump_enabled_p ())
	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
			     "conditional reduction prevents the use"
			     " of partial vectors\n");

OK with that change, thanks.

Richard
diff mbox series

Patch

diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 40e2664f93b..c23520aceab 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -9968,11 +9968,16 @@  vectorizable_condition (vec_info *vinfo,
 	  return false;
 	}
 
-      if (loop_vinfo
-	  && LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo)
-	  && reduction_type == EXTRACT_LAST_REDUCTION)
-	vect_record_loop_mask (loop_vinfo, &LOOP_VINFO_MASKS (loop_vinfo),
-			       ncopies * vec_num, vectype, NULL);
+      if (loop_vinfo && for_reduction
+	  && LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo))
+	{
+	  if (reduction_type == EXTRACT_LAST_REDUCTION)
+	    vect_record_loop_mask (loop_vinfo, &LOOP_VINFO_MASKS (loop_vinfo),
+				   ncopies * vec_num, vectype, NULL);
+	  /* Extra inactive lanes should be safe for vect_nested_cycle.  */
+	  else if (STMT_VINFO_DEF_TYPE (reduc_info) != vect_nested_cycle)
+	    LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;
+	}
 
       STMT_VINFO_TYPE (stmt_info) = condition_vec_info_type;
       vect_model_simple_cost (vinfo, stmt_info, ncopies, dts, ndts, slp_node,