[PING] [PATCH] _Cilk_for for C and C++

[Jakub Jelinek]

On Wed, Feb 12, 2014 at 03:14:23PM +0000, Iyer, Balaji V wrote:
> > The testcase is GPL as the original libgomp.c++/for-1.C testcase, so sure.
> > Perhaps it would be much better though if instead of having a compile time
> > testcase you'd just do what libgomp.c++/for-1.C does, just replace all the
> > #pragma omp parallel for in there with _Cilk_for and turn it into a runtime
> > testcase.
> > 

> I really don't want to do that because I don't think there is a 1:1
> match-up between the rules of #pragma omp for and _Cilk_for.

But there is nothing OpenMP specific on any of the tests, all could as well
be tested by removing all the #pragma omp ... lines and just be tested as
normal C+++ loops.  Is there anything that _Cilk_for wouldn't handle?

IMNSHO if you remove all the #pragma omp parallel for lines (even with any
clauses it sometimes has) and replace for with _Cilk_for on the following
line, you should have a valid Cilk+ program.

Only f11 would need to be changed from:
#pragma omp parallel
  {
#pragma omp for nowait
    for (T i = x; i <= y; i += 3)
      baz (i);
#pragma omp single
    {
      T j = y + 3;
      baz (j);
    }
  }
to say:
  _Cilk_for (T i = x; i <= y; i += 3)
    baz (i);
  {
    T j = y + 3;
    baz (j);
  }
so that it performs the same thing.

> > First a minor nit, there is extra newline before _Cilk_for:
> >           newline_and_indent (buffer, spc);
> >           if (flag_cilkplus
> >               && gimple_omp_for_kind (gs) == GF_OMP_FOR_KIND_CILKFOR)
> >             pp_string (buffer, "_Cilk_for (");
> >           else
> >             pp_string (buffer, "for ("); I guess for _Cilk_for collapse is never > 1,
> > right?  If that is the case, then perhaps you should move the
> > newline_and_indent (buffer, spc); call into the else block.
> > 
> 
> OK. I will fix this and send you a patch? 

Sure.

> > More importantly, what is retval.1?  I'd expect you should be using retval.0
> > there and have it also as firstprivate(retval.0) on the parallel.
> > In *.omplower dump I actually see:
> >         retval.0 = operator-<int> (D.2885, &i); ...
> >                             retval.1 = operator-<int> (D.2888, &i); i.e. operator-<int> is
> > called twice.
> > 
> 
> Yes, one is for the if-clause and one is for the condition. It really doesn't matter because we get of the stuff in the condition and replace with our own for loop with something like the for-loop shown  below. So retval1 doesn't come into picture. It is only alive from parser to the expand_cilk_for function.
> 
> For (i = low; i < high; i++)
> {
> 	<_Cilk_for_body>
> }

No, it really does matter.  Just look at the *.optimized dump with -O0 -fcilkplus:

  retval.0_4 = operator-<int> (_3, &i);
in _Z3foo1JIiE function and
  _4 = .omp_data_i_2(D)->j;
  _5 = J<int>::end (_4);
  retval.1_6 = operator-<int> (_5, &i);
  retval.3_7 = retval.1_6;
in _Z3foo1JIiE._cilk_for_fn.0.  All the 4 statements are dead, you really
shouldn't emit them, even when optimizing, if e.g. the operator- isn't
inline, g++ won't be able to optimize it away.

	Jakub