On Tue, 14 May 2013, Jakub Jelinek wrote:
> 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.
Works for me.
> > 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.
But don't we now know the loop (it's header and possibly its latch)
and so we can simply add_loop here?
> > > --- 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.
Might not be trivial - the dependency whould have to be of "known"
type and the distance vector maybe safelen+1 (which would be
not exactly correct I think, but there isn't sth like "at least"
safelen+1). So eventually it needs to be an "unknown" dependency
still with a new interface of a "at least" distance result.
Richard.
