On 21 September 2016 at 09:16, Richard Biener <rguent...@suse.de> wrote:
> On Tue, 20 Sep 2016, Bernd Edlinger wrote:
>
>> On 09/20/16 09:41, Richard Biener wrote:
>> > On Mon, 19 Sep 2016, Bernd Edlinger wrote:
>> >
>> >> 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,
>> >
>> > Yes. I'm not sure group_size or group_gap_adj are used in the
>> > slp && ! SLP_TREE_LOAD_PERMUTATION (slp_node).exists () case but moving
>> > the computation up before we re-set first_stmt is probably a good idea.
>> >
>> >> 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....
>> >
>> > See above - it's eventually unused. The load/store vectorization code
>> > is quite twisted ;)
>> >
>>
>> Agreed.
>>
>> Here is the new version of the patch:
>>
>> Moved the goups_size up, and everything works fine.
>>
>> Removed the parameter from get_group_alias_ptr_type.
>>
>> I think in the case, where first_stmt is not set to
>> GROUP_FIRST_ELEMENT (stmt_info) but directly to stmt,
>> it is likely somewhere in a list, so it is not necessary
>> to walk the GROUP_NEXT_ELEMENT, so I would like to call
>> reference_alias_ptr_type directly in that case.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk and gcc-6 branch?
>
> Ok for trunk and gcc-6 branch after a while.
>
> Richard.
Hi,
The new testcase pr77550.C fails on arm:
/testsuite/g++.dg/pr77550.C:39:43: error: 'operator new' takes type
'size_t' ('unsigned int') as first parameter [-fpermissive]
compiler exited with status 1
Christophe
