Re: [PATCH] Fix PR tree-optimization/77550

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

On 09/19/16 11:25, Richard Biener wrote:
> On Sun, 18 Sep 2016, Bernd Edlinger wrote:
>
>> Hi,
>>
>> this PR shows that in vectorizable_store and vectorizable_load
>> as well, the vector access always uses the first dr as the alias
>> type for the whole access.  But that is not right, if they are
>> different types, like in this example.
>>
>> So I tried to replace all reference_alias_ptr_type (DR_REF (first_dr))
>> by an alias type that is correct for all references in the whole
>> access group.  With this patch we fall back to ptr_type_node, which
>> can alias anything, if the group consists of different alias sets.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk and gcc-6-branch?
>
> +/* Function get_group_alias_ptr_type.
> +
> +   Return the alias type for the group starting at FIRST_STMT
> +   containing GROUP_SIZE elements.  */
> +
> +static tree
> +get_group_alias_ptr_type (gimple *first_stmt, int group_size)
> +{
> +  struct data_reference *first_dr, *next_dr;
> +  gimple *next_stmt;
> +  int i;
> +
> +  first_dr = STMT_VINFO_DATA_REF (vinfo_for_stmt (first_stmt));
> +  next_stmt = GROUP_NEXT_ELEMENT (vinfo_for_stmt (first_stmt));
> +  for (i = 1; i < group_size && next_stmt; i++)
> +    {
>
>
> there is no need to pass in group_size, it's enough to walk
> GROUP_NEXT_ELEMENT until it becomes NULL.
>
> Ok with removing the redundant arg.
>
> Thanks,
> Richard.
>

Hmmm, I'm afraid this needs one more iteration.


I tried first to check if there are no stmts after the group_size
and noticed there are cases when group_size is 0,
for instance in gcc.dg/torture/pr36244.c.

I think there is a bug in vectorizable_load, here:

  if (grouped_load)
    {
      first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
       /* For SLP vectorization we directly vectorize a subchain
          without permutation.  */
       if (slp && ! SLP_TREE_LOAD_PERMUTATION (slp_node).exists ())
         first_stmt = ;

       group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt));

       = 0, and even worse:

       group_gap_adj = vf * group_size - nunits * vec_num;

       = -4 !

apparently GROUP_SIZE is only valid on the GROUP_FIRST_ELEMENT,
while it may be 0 on SLP_TREE_SCALAR_STMTS (slp_node)[0]

moving the GROUP_SIZE up before first_stmt is overwritten
results in no different code....


Bernd.