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?
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))
{
