Richard Guenther <rguent...@suse.de> wrote on 05/07/2011 12:35:24 PM:
>
> On Tue, 5 Jul 2011, Ira Rosen wrote:
>
> > Richard Guenther <rguent...@suse.de> wrote on 04/07/2011 03:30:59 PM:
> > > >
> > > > Richard Guenther <rguent...@suse.de> wrote on 04/07/2011 02:38:50
PM:
> > > >
> > > > > Handling of negative steps broke one of the many asserts in
> > > > > the vectorizer. The following patch drops one that I can't
> > > > > make sense of. I think all asserts need comments - especially
> > > > > this one would, as I can't see why using vf is correct to
> > > > > test against and not nelements (and why <= vf and not < vf).
> > > >
> > > > There is an explanation 10 rows above the assert. It doesn't make
sense
> > to
> > > > peel more than vf iterations (and not nelements, since for the case
of
> > > > multiple types it may help to align more data-refs - see the
comment in
> > the
> > > > code). IIRC <= is for the case of aligned access, but I am not sure
> > about
> > > > that, so maybe you are right.
> > > >
> > > > I don't see how it is related to negative steps.
> > > >
> > > > I think that the real reason for this failure is that the loads are
> > > > actually irrelevant (hence, vf=4 that doesn't take char loads into
> > > > account), but we don't check that when we analyze data-refs. So, in
my
> > > > opinion, the proper fix will add such check.
> > >
> > > The following also works for me:
> > >
> > > Index: tree-vect-data-refs.c
> > > ===================================================================
> > > --- tree-vect-data-refs.c (revision 175802)
> > > +++ tree-vect-data-refs.c (working copy)
> > > @@ -1495,6 +1495,9 @@ vect_enhance_data_refs_alignment (loop_v
> > > stmt = DR_STMT (dr);
> > > stmt_info = vinfo_for_stmt (stmt);
> > >
> > > + if (!STMT_VINFO_RELEVANT (stmt_info))
> > > + continue;
> > > +
> > > /* For interleaving, only the alignment of the first access
> > > matters. */
> > > if (STMT_VINFO_STRIDED_ACCESS (stmt_info)
> > >
> > > does that look better or do you propose to clean the datarefs
> > > vector from those references?
> >
> > Well, this is certainly enough to fix the PR.
> > I am not sure if we can just remove these data-refs from the dependence
> > checks. After that all the alignment and access checks are at least
> > redundant.
>
> Ok. The following is what I have tested for this PR and also for
> PR49628.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?
Yes.
Thanks,
Ira
>
> Thanks,
> Richard.
>
> 2011-07-04 Richard Guenther <rguent...@suse.de>
>
> PR tree-optimization/49518
> PR tree-optimization/49628
> * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Skip
> irrelevant and invariant data-references.
> (vect_analyze_data_ref_access): For invariant loads clear the
> group association.
>
> * g++.dg/torture/pr49628.C: New testcase.
> * gcc.dg/torture/pr49518.c: Likewise.
>
> Index: gcc/testsuite/g++.dg/torture/pr49628.C
> ===================================================================
> *** gcc/testsuite/g++.dg/torture/pr49628.C (revision 0)
> --- gcc/testsuite/g++.dg/torture/pr49628.C (revision 0)
> ***************
> *** 0 ****
> --- 1,37 ----
> + /* { dg-do compile } */
> +
> + #include <vector>
> +
> + template <int rank, int dim> class Tensor;
> + template <int dim> class Tensor<1,dim> {
> + public:
> + explicit Tensor (const bool initialize = true);
> + Tensor (const Tensor<1,dim> &);
> + Tensor<1,dim> & operator = (const Tensor<1,dim> &);
> + double values[(dim!=0) ? (dim) : 1];
> + };
> + template <int dim>
> + inline Tensor<1,dim> & Tensor<1,dim>::operator = (const Tensor<1,dim>
&p)
> + {
> + for (unsigned int i=0; i<dim; ++i)
> + values[i] = p.values[i];
> + };
> + template <int dim> class Quadrature {
> + public:
> + const unsigned int n_quadrature_points;
> + };
> + class MappingQ1
> + {
> + class InternalData {
> + public:
> + std::vector<Tensor<1,3> > shape_derivatives;
> + unsigned int n_shape_functions;
> + };
> + void compute_data (const Quadrature<3> &quadrature, InternalData
&data)
> + const;
> + };
> + void MappingQ1::compute_data (const Quadrature<3> &q, InternalData
> &data) const
> + {
> + const unsigned int n_q_points = q.n_quadrature_points;
> + data.shape_derivatives.resize(data.n_shape_functions * n_q_points);
> + }
> Index: gcc/testsuite/gcc.dg/torture/pr49518.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/torture/pr49518.c (revision 0)
> --- gcc/testsuite/gcc.dg/torture/pr49518.c (revision 0)
> ***************
> *** 0 ****
> --- 1,19 ----
> + /* { dg-do compile } */
> +
> + int a, b;
> + struct S { unsigned int s, t, u; } c, d = { 0, 1, 0 };
> +
> + void
> + test (unsigned char z)
> + {
> + char e[] = {0, 0, 0, 0, 1};
> + for (c.s = 1; c.s; c.s++)
> + {
> + b = e[c.s];
> + if (a)
> + break;
> + b = z >= c.u;
> + if (d.t)
> + break;
> + }
> + }
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> --- gcc/tree-vect-data-refs.c (revision 175802)
> +++ gcc/tree-vect-data-refs.c (working copy)
> @@ -1495,12 +1495,19 @@ vect_enhance_data_refs_alignment (loop_v
> stmt = DR_STMT (dr);
> stmt_info = vinfo_for_stmt (stmt);
>
> + if (!STMT_VINFO_RELEVANT (stmt_info))
> + continue;
> +
> /* For interleaving, only the alignment of the first access
> matters. */
> if (STMT_VINFO_STRIDED_ACCESS (stmt_info)
> && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
> continue;
>
> + /* For invariant accesses there is nothing to enhance. */
> + if (integer_zerop (DR_STEP (dr)))
> + continue;
> +
> supportable_dr_alignment = vect_supportable_dr_alignment (dr,
true);
> do_peeling = vector_alignment_reachable_p (dr);
> if (do_peeling)
> @@ -2304,7 +2311,10 @@ vect_analyze_data_ref_access (struct dat
>
> /* Allow invariant loads in loops. */
> if (loop_vinfo && dr_step == 0)
> - return DR_IS_READ (dr);
> + {
> + GROUP_FIRST_ELEMENT (vinfo_for_stmt (stmt)) = NULL;
> + return DR_IS_READ (dr);
> + }
>
> if (loop && nested_in_vect_loop_p (loop, stmt))
> {
