Re: [PATCH] tree-optimization/101801 - remove vect_worthwhile_without_simd_p

Richard Biener rguenther@suse.de
Fri Aug 6 14:24:15 GMT 2021


On August 6, 2021 3:47:44 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> This removes the cost part of vect_worthwhile_without_simd_p, retaining
>> only the correctness bits.  The reason is that the cost heuristic
>> do not properly account for SLP plus the check whether "without simd"
>> applies misfires for AVX512 mask vectors at the moment, leading to
>> missed vectorizations there.
>>
>> Any costing decision should take place in the cost modeling, no
>> single stmt is to disable all vectorization on its own.
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed to trunk.
>>
>> 2021-08-06  Richard Biener  <rguenther@suse.de>
>>
>> 	PR tree-optimization/101801
>> 	* tree-vectorizer.h (vect_worthwhile_without_simd_p): Rename...
>> 	(vect_can_vectorize_without_simd_p): ... to this.
>> 	* tree-vect-loop.c (vect_worthwhile_without_simd_p): Rename...
>> 	(vect_can_vectorize_without_simd_p): ... to this and fold
>> 	in vect_min_worthwhile_factor.
>> 	(vect_min_worthwhile_factor): Remove.
>> 	(vectorizable_reduction): Adjust and remove the cost part.
>> 	* tree-vect-stmts.c (vectorizable_shift): Likewise.
>> 	(vectorizable_operation): Likewise.
>> ---
>>  gcc/tree-vect-loop.c  | 43 +++++++------------------------------------
>>  gcc/tree-vect-stmts.c | 26 ++------------------------
>>  gcc/tree-vectorizer.h |  2 +-
>>  3 files changed, 10 insertions(+), 61 deletions(-)
>>
>> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
>> index 1e21fe6b13d..37c7daa7f9e 100644
>> --- a/gcc/tree-vect-loop.c
>> +++ b/gcc/tree-vect-loop.c
>> @@ -7227,24 +7227,13 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
>>            if (dump_enabled_p ())
>>              dump_printf (MSG_NOTE, "op not supported by target.\n");
>>  	  if (maybe_ne (GET_MODE_SIZE (vec_mode), UNITS_PER_WORD)
>> -	      || !vect_worthwhile_without_simd_p (loop_vinfo, code))
>> +	      || !vect_can_vectorize_without_simd_p (code))
>>  	    ok = false;
>>  	  else
>>  	    if (dump_enabled_p ())
>>  	      dump_printf (MSG_NOTE, "proceeding using word mode.\n");
>>          }
>>  
>> -      /* Worthwhile without SIMD support?  */
>> -      if (ok
>> -	  && !VECTOR_MODE_P (TYPE_MODE (vectype_in))
>> -	  && !vect_worthwhile_without_simd_p (loop_vinfo, code))
>> -        {
>> -          if (dump_enabled_p ())
>> -	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> -			     "not worthwhile without SIMD support.\n");
>> -	  ok = false;
>> -        }
>> -
>>        /* lane-reducing operations have to go through vect_transform_reduction.
>>           For the other cases try without the single cycle optimization.  */
>>        if (!ok)
>> @@ -7948,46 +7937,28 @@ vectorizable_phi (vec_info *,
>>  }
>>  
>>  
>> -/* Function vect_min_worthwhile_factor.
>> +/* Return true if we can emulate CODE on an integer mode representation
>> +   of a vector.  */
>>  
>> -   For a loop where we could vectorize the operation indicated by CODE,
>> -   return the minimum vectorization factor that makes it worthwhile
>> -   to use generic vectors.  */
>> -static unsigned int
>> -vect_min_worthwhile_factor (enum tree_code code)
>> +bool
>> +vect_can_vectorize_without_simd_p (tree_code code)
>>  {
>>    switch (code)
>>      {
>>      case PLUS_EXPR:
>>      case MINUS_EXPR:
>>      case NEGATE_EXPR:
>> -      return 4;
>> -
>>      case BIT_AND_EXPR:
>>      case BIT_IOR_EXPR:
>>      case BIT_XOR_EXPR:
>>      case BIT_NOT_EXPR:
>> -      return 2;
>> +      return true;
>>  
>>      default:
>> -      return INT_MAX;
>> +      return false;
>>      }
>>  }
>>  
>> -/* Return true if VINFO indicates we are doing loop vectorization and if
>> -   it is worth decomposing CODE operations into scalar operations for
>> -   that loop's vectorization factor.  */
>> -
>> -bool
>> -vect_worthwhile_without_simd_p (vec_info *vinfo, tree_code code)
>> -{
>> -  loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo);
>> -  unsigned HOST_WIDE_INT value;
>> -  return (loop_vinfo
>> -	  && LOOP_VINFO_VECT_FACTOR (loop_vinfo).is_constant (&value)
>> -	  && value >= vect_min_worthwhile_factor (code));
>> -}
>> -
>>  /* Function vectorizable_induction
>>  
>>     Check if STMT_INFO performs an induction computation that can be vectorized.
>> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
>> index 94bdb74ea8d..5b94d41e292 100644
>> --- a/gcc/tree-vect-stmts.c
>> +++ b/gcc/tree-vect-stmts.c
>> @@ -5685,24 +5685,13 @@ vectorizable_shift (vec_info *vinfo,
>>        /* Check only during analysis.  */
>>        if (maybe_ne (GET_MODE_SIZE (vec_mode), UNITS_PER_WORD)
>>  	  || (!vec_stmt
>> -	      && !vect_worthwhile_without_simd_p (vinfo, code)))
>> +	      && !vect_can_vectorize_without_simd_p (code)))
>>          return false;
>>        if (dump_enabled_p ())
>>          dump_printf_loc (MSG_NOTE, vect_location,
>>                           "proceeding using word mode.\n");
>>      }
>>  
>> -  /* Worthwhile without SIMD support?  Check only during analysis.  */
>> -  if (!vec_stmt
>> -      && !VECTOR_MODE_P (TYPE_MODE (vectype))
>> -      && !vect_worthwhile_without_simd_p (vinfo, code))
>> -    {
>> -      if (dump_enabled_p ())
>> -        dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> -                         "not worthwhile without SIMD support.\n");
>> -      return false;
>> -    }
>> -
>>    if (!vec_stmt) /* transformation not required.  */
>>      {
>>        if (slp_node
>
>It looks from vect_min_worthwhile_factor like this shift stuff
>was/is effectively dead code.  Maybe it started life as a copy of
>vectorizable_operation or something?

Yeah, looks like so. I did trace back the original introduction and there was no vectirizable_shift at that point. 

Richard. 

Richard. 


>Thanks,
>Richard
>
>> @@ -6094,24 +6083,13 @@ vectorizable_operation (vec_info *vinfo,
>>                           "op not supported by target.\n");
>>        /* Check only during analysis.  */
>>        if (maybe_ne (GET_MODE_SIZE (vec_mode), UNITS_PER_WORD)
>> -	  || (!vec_stmt && !vect_worthwhile_without_simd_p (vinfo, code)))
>> +	  || (!vec_stmt && !vect_can_vectorize_without_simd_p (code)))
>>          return false;
>>        if (dump_enabled_p ())
>>  	dump_printf_loc (MSG_NOTE, vect_location,
>>                           "proceeding using word mode.\n");
>>      }
>>  
>> -  /* Worthwhile without SIMD support?  Check only during analysis.  */
>> -  if (!VECTOR_MODE_P (vec_mode)
>> -      && !vec_stmt
>> -      && !vect_worthwhile_without_simd_p (vinfo, code))
>> -    {
>> -      if (dump_enabled_p ())
>> -        dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> -                         "not worthwhile without SIMD support.\n");
>> -      return false;
>> -    }
>> -
>>    int reduc_idx = STMT_VINFO_REDUC_IDX (stmt_info);
>>    vec_loop_masks *masks = (loop_vinfo ? &LOOP_VINFO_MASKS (loop_vinfo) : NULL);
>>    internal_fn cond_fn = get_conditional_internal_fn (code);
>> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
>> index 5571b3cce3b..de0ecf86478 100644
>> --- a/gcc/tree-vectorizer.h
>> +++ b/gcc/tree-vectorizer.h
>> @@ -2061,7 +2061,7 @@ extern bool vectorizable_lc_phi (loop_vec_info, stmt_vec_info,
>>  				 gimple **, slp_tree);
>>  extern bool vectorizable_phi (vec_info *, stmt_vec_info, gimple **, slp_tree,
>>  			      stmt_vector_for_cost *);
>> -extern bool vect_worthwhile_without_simd_p (vec_info *, tree_code);
>> +extern bool vect_can_vectorize_without_simd_p (tree_code);
>>  extern int vect_get_known_peeling_cost (loop_vec_info, int, int *,
>>  					stmt_vector_for_cost *,
>>  					stmt_vector_for_cost *,



More information about the Gcc-patches mailing list