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
