On Tue, May 14, 2013 at 11:28:43AM +0200, Richard Biener wrote:
> > +  /* If non-NULL, an INTEGER_CST, where the user asserted that for any
> > +     I in [ 0, nb_iterations ) and for any J in
> > +     [ I, min ( I + safelen, nb_iterations ) ), the Ith and Jth iterations
> > +     of the loop can be safely evaluated concurrently.  */
> > +  tree safelen;
> 
> Can you make this a double_int (or a HOST_WIDE_INT or an int) instead 
> please?  It should map to data-dependence analysis distance vectors
> which currently is a vector of 'int'.

If all we care about is int, it can be int.  Infinity is when
#pragma omp simd
doesn't contain any simdlen clause, or when Cilk+
#pragma simd
doesn't contain any vectorlength or vectorlengthfor clauses.
So, shall we assign INT_MAX for infinity?  At least the vectorizer
certainly doesn't care about anything beyond MAX_VECTORIZATION_FACTOR.
And I can just map any explicit safelen in the source larger than INT_MAX
as INT_MAX.

> Is there a magic value to tell safelen is "infinity"?  As I read
> above safelen == 0 would mean all iterations are dependent.  Are
> negative safelen values well-defined?  The comment doesn't seem
> to disallow them.

The FEs disallow safelen <= 0 or non-constant.

> Also make sure to copy the field in copy_loop_info and stream
> it in output/input_cfg in lto-streamer-in/out.c.

Ok.

> > +  if (!broken_loop)
> > +    {
> > +      struct loop *loop;
> > +      calculate_dominance_info (CDI_DOMINATORS);
> > +      fix_loop_structure (NULL);
> 
> Ick.  Didn't I properly add loops everywhere?

The loop was previously containing EDGE_ABNORMAL edges (that is something
to prevent any optimizations on those until ompexp had a chance to deal with
those), so there is no loop at all, just the loop->num == 0 for the whole
function if #pragma omp simd appears outside of loops and doesn't contain
any loops inside of its body.

> > --- gcc/tree-vectorizer.c.jj        2013-05-13 16:49:03.000000000 +0200
> > +++ gcc/tree-vectorizer.c   2013-05-13 20:44:58.721863725 +0200
> > @@ -101,7 +101,8 @@ vectorize_loops (void)
> >       than all previously defined loops.  This fact allows us to run
> >       only over initial loops skipping newly generated ones.  */
> >    FOR_EACH_LOOP (li, loop, 0)
> > -    if (optimize_loop_nest_for_speed_p (loop))
> > +    if ((flag_tree_vectorize && optimize_loop_nest_for_speed_p (loop))
> > +   || loop->safelen)
> 
> So you vectorize all loops with a safelen?  I'd say this warrants an
> extra flag in struct loop, force_vect.

Ok.

> > @@ -225,7 +225,8 @@ tree_vectorize (void)
> >  static bool
> >  gate_tree_vectorize (void)
> >  {
> > -  return flag_tree_vectorize;
> > +  return flag_tree_vectorize
> > +    || (flag_openmp && !global_options_set.x_flag_tree_vectorize);
> 
> And a flag in cfun here, whether any loop has force_vect set (or
> a flag in current_loops)

Ok.

> Rather than looking at safelen from data-dependence analysis consumers
> data-dependence analysis itself should use the information.  Which
> is why I'd like the 'safelen' thing to map to the distance vector
> representation of dependence analysis.

Will try.

        Jakub

Reply via email to