Re: [PATCH] Reduction support for parloop, OMP_ATOMIC Changes

[Razya Ladelsky <RAZYA at il dot ibm dot com>]

Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote on 30/09/2007 20:53:04:

> Hello,
> 
> Overall, the patch looks fine to me.  You will of course need a separate
> review for the changes outside of loop optimizer.  It might make things
> easier to submit the OMP expansion changes for review separately.
> 

Zdenek,
Thanks for your quick review :)
I'll make the proper changes to the code.

Diego,
I could submit the OMP expansion changes as a separate patch.
Do you prefer that I do so?

> 
> >    * testsite/gcc.dg/autopar: New directory.
> >    * testsite/gcc.dg/autopar/autopar.exp: New driver.
> >    * testsite/gcc.dg/autopar/reduc-1.c: New test.
> 
> I am not sure whether we want to create a new directory for the
> parallelization testcases -- we certainly do not do that for every
> optimization.
> 

I can move the testcases to tree-ssa directory.

> 
> > + static int
> > + create_phi_for_local_result (void **slot, void *data)
> > + {
> > +   struct reduction_info *reduc = *slot;
> > +   struct loop *loop = data;
> > +   edge e;
> > +   tree new_phi;
> > +   basic_block store_bb;
> > +   tree local_res;
> > + 
> > +   store_bb = FALLTHRU_EDGE (loop->latch)->dest;
> 
> This looks wrong.  Under normal circumstances, FALLTHRU_EDGE
> (loop->latch)->dest == loop->header, which does not seem to be the case
> here.  I assume that we are now in some temporary state where we
> inserted the exit condition to the loop latch.  But in that case,
> loop latch will have two outgoing edges (to the loop header and
> the exit), and neither of them will be marked with EDGE_FALLTHRU flag.
> 

This code is called after create_parallel_loop, where it is set:

  single_succ_edge (loop->latch)->flags = 0;
  end = make_edge (loop->latch, ex_bb, EDGE_FALLTHRU);

ok?

> 
> > + /* Create the atomic operation at the join point of the threads.  */
> > + static void
> > + create_call_for_reduction (struct loop *loop, struct clsn_data 
*clsn_data)
> > + {
> > +   htab_traverse (reduction_list, create_phi_for_local_result, loop);
> > +   clsn_data->load_bb = FALLTHRU_EDGE (loop->latch)->dest;
> 
> Again, FALLTHRU_EDGE (loop->latch) seems wrong.
> 

Same.