Re: [11a/n] Avoid retrying with the same vector modes

[Richard Sandiford <richard dot sandiford at arm dot com>]

Richard Biener <richard.guenther@gmail.com> writes:
> On Tue, Nov 5, 2019 at 9:25 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Patch 12/n makes the AArch64 port add four entries to
>> autovectorize_vector_modes.  Each entry describes a different
>> vector mode assignment for vector code that mixes 8-bit, 16-bit,
>> 32-bit and 64-bit elements.  But if (as usual) the vector code has
>> fewer element sizes than that, we could end up trying the same
>> combination of vector modes multiple times.  This patch adds a
>> check to prevent that.
>>
>> As before: each patch tested individually on aarch64-linux-gnu and the
>> series as a whole on x86_64-linux-gnu.
>>
>>
>> 2019-11-04  Richard Sandiford  <richard.sandiford@arm.com>
>>
>> gcc/
>>         * tree-vectorizer.h (vec_info::mode_set): New typedef.
>>         (vec_info::used_vector_mode): New member variable.
>>         (vect_chooses_same_modes_p): Declare.
>>         * tree-vect-stmts.c (get_vectype_for_scalar_type): Record each
>>         chosen vector mode in vec_info::used_vector_mode.
>>         (vect_chooses_same_modes_p): New function.
>>         * tree-vect-loop.c (vect_analyze_loop): Use it to avoid trying
>>         the same vector statements multiple times.
>>         * tree-vect-slp.c (vect_slp_bb_region): Likewise.
>>
>> Index: gcc/tree-vectorizer.h
>> ===================================================================
>> --- gcc/tree-vectorizer.h       2019-11-05 10:48:11.246092351 +0000
>> +++ gcc/tree-vectorizer.h       2019-11-05 10:57:41.662071145 +0000
>> @@ -298,6 +298,7 @@ typedef std::pair<tree, tree> vec_object
>>  /* Vectorizer state common between loop and basic-block vectorization.  */
>>  class vec_info {
>>  public:
>> +  typedef hash_set<int_hash<machine_mode, E_VOIDmode, E_BLKmode> > mode_set;
>>    enum vec_kind { bb, loop };
>>
>>    vec_info (vec_kind, void *, vec_info_shared *);
>> @@ -335,6 +336,9 @@ typedef std::pair<tree, tree> vec_object
>>    /* Cost data used by the target cost model.  */
>>    void *target_cost_data;
>>
>> +  /* The set of vector modes used in the vectorized region.  */
>> +  mode_set used_vector_modes;
>> +
>>    /* The argument we should pass to related_vector_mode when looking up
>>       the vector mode for a scalar mode, or VOIDmode if we haven't yet
>>       made any decisions about which vector modes to use.  */
>> @@ -1615,6 +1619,7 @@ extern tree get_related_vectype_for_scal
>>  extern tree get_vectype_for_scalar_type (vec_info *, tree);
>>  extern tree get_mask_type_for_scalar_type (vec_info *, tree);
>>  extern tree get_same_sized_vectype (tree, tree);
>> +extern bool vect_chooses_same_modes_p (vec_info *, machine_mode);
>>  extern bool vect_get_loop_mask_type (loop_vec_info);
>>  extern bool vect_is_simple_use (tree, vec_info *, enum vect_def_type *,
>>                                 stmt_vec_info * = NULL, gimple ** = NULL);
>> Index: gcc/tree-vect-stmts.c
>> ===================================================================
>> --- gcc/tree-vect-stmts.c       2019-11-05 10:48:11.242092379 +0000
>> +++ gcc/tree-vect-stmts.c       2019-11-05 10:57:41.662071145 +0000
>> @@ -11235,6 +11235,10 @@ get_vectype_for_scalar_type (vec_info *v
>>                                                       scalar_type);
>>    if (vectype && vinfo->vector_mode == VOIDmode)
>>      vinfo->vector_mode = TYPE_MODE (vectype);
>> +
>> +  if (vectype)
>> +    vinfo->used_vector_modes.add (TYPE_MODE (vectype));
>> +
>
> Do we actually end up _using_ all types returned by this function?

No, not all of them, so it's a bit crude.  E.g. some types might end up
not being relevant after pattern recognition, or after we've made a
final decision about which parts of an address calculation to include
in a gather or scatter op.  So we can still end up retrying the same
thing even after the patch.

The problem is that we're trying to avoid pointless retries on failure
as well as success, so we could end up stopping at arbitrary points.
I wasn't sure where else to handle this.

Thanks,
Richard