[Bug tree-optimization/58028] [4.9 Regression] Several failures in libgomp.graphite after revision 200946

["rguenther at suse dot de" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58028

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 29 Jan 2014, grosser at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58028
> 
> --- Comment #9 from Tobias Grosser <grosser at gcc dot gnu.org> ---
> (In reply to rguenther@suse.de from comment #8)
> > On Wed, 29 Jan 2014, dominiq at lps dot ens.fr wrote:
> > 
> > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58028
> > > 
> > > --- Comment #7 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> > > > The testsuite failures are.  We have to do sth about them.
> > > 
> > > Revert r200946?
> > 
> > Probably - the
> > 
> > +       * graphite-dependences.c (carries_deps): Do not assume the 
> > schedule is
> > +         in 2D + 1 form.
> > 
> > part looks wrong.
> > 
> > -  idx = 2 * depth + 1;
> > -  for (i = 0; i < idx; i++)
> > +  for (i = 0; i < depth - 1; i++)
> > 
> > we now iterate over less dimensions than before.
> 
> Only in cases where the schedule is not in 2D + 1 form. In case the schedule
> is in 2D + 1 form 
> 
> int scheduling_dim = isl_set_n_dim (domain);
> 
> will be equal to 2 * depth + 1.
> 
> > I'd say we should simply check whether the loop _is_ in 2D + 1 form
> > at
> > 
> > +  isl_set *domain = isl_set_from_cloog_domain (stmt->domain);
> > +  int scheduling_dim = isl_set_n_dim (domain);
> > +
> >    if (flag_loop_parallelize_all
> > -      && loop_is_parallel_p (loop, bb_pbb_mapping, level))
> > +      && loop_is_parallel_p (loop, bb_pbb_mapping, scheduling_dim))
> >      loop->can_be_parallel = true;
> >  
> > thus
> > 
> >    if (flag_loop_parallelize_all
> >        && scheduling_dim == 2 * level + 1
> >        && loop_is_parallel_p (loop, bb_pbb_mapping, level)
> >      loop->can_be_parallel = true;
> 
> The change you propose seems conservatively correct, as in that loops that
> are not in 2D + 1 form are not detected to be parallel. This change may hide
> the bug, but I don't see any bug it solves.

It may solve the fallout from your change - I don't have enough
graphite knowledge to fix a real bug there ;)

> > no time to check whether reverting the other hunk plus this will
> > resolve the bug the revision fixed and restores the testcases.
> 
> I assume it will fix the crash, but it will not detect parallellism in none
> 2D+1 loops, something the isl scheduler happily creates.

Sure.  We'd be much happier if the real bug was fixed, maybe you can
give it a try?  The

FAIL: libgomp.graphite/force-parallel-4.c execution test
FAIL: libgomp.graphite/force-parallel-4.c scan-tree-dump-times graphite "2 
loops
 carried no dependency" 1

FAIL suggests that we miss some dependency now.