> -----Original Message----- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Jakub Jelinek > Sent: Friday, February 7, 2014 9:03 AM > To: Iyer, Balaji V > Cc: 'Jason Merrill'; 'Jeff Law'; 'Aldy Hernandez'; 'gcc-patches@gcc.gnu.org'; > 'r...@redhat.com' > Subject: Re: [PING] [PATCH] _Cilk_for for C and C++ > > On Wed, Feb 05, 2014 at 05:27:26AM +0000, Iyer, Balaji V wrote: > > Attached, please find a fixed patch (diff.txt) that will do as you > requested (model _Cilk_for like a #pragma omp parallel for). Along with this, > I have also attached two Changelog entries (1 for C and 1 for C++). > > It passes all the tests on my x86_64 box (both 32 and 64 bit modes) > and does not affect any other tests in the testsuite. > > Is this Ok for trunk? > > A step in the right direction, but I still see issues just from looking at the > *.gimple dump: > > For the first testcase, I see: > iter = std::vector<int>::begin (array); [return slot optimization] > iter.1 = iter; > D.13615 = std::vector<int>::end (array); [return slot > optimization] > try > { > retval.0 = __gnu_cxx::operator-<int*, std::vector<int> > > (&D.13615, > &iter); > } > finally > { > D.13615 = {CLOBBER}; > } > #pragma omp parallel schedule(cilk-for grain,0) if(retval.0) > #shared(iter.1) shared(D.13632) shared(D.13615) shared(iter) > { > difference_type retval.2; > const difference_type D.13633; > int D.13725; > struct __normal_iterator & D.13726; > bool retval.3; > int & D.13728; > int D.13729; > int & D.13732; > > iter = iter.1; > private(D.13631) > _Cilk_for (D.13631 = 0; D.13631 != retval.2; D.13631 = > D.13631 + 1) > D.13725 = D.13631 - D.13632; > > So, the issues I see: > 1) what is iter.1, why do you have it at all, and, after all, the iterator is > a class > that needs to be constructed/destructed in the general way, so creating any > further copies of something is both costly and undesirable >
Well, to get the loop count, I need to calculate it using operator-(array.end (), &iter). Now, if I do that iter is already set. I need to reset iter back to the original one (array.begin ()) in the child function. This is why I used a temporary variable called iter1. > 2) the schedule clause doesn't belong on the omp parallel, but on the > _Cilk_for > What if grain is a variable say "x"? If I have it in the _Cilk_for, then won't it create omp_data_i->x. That is not correct. It should just emit "x." But let me look into this to make sure... > 3) iter should be firstprivate, and there should be no explicit private var > with > assignment during gimplification, just handle it like any other firstprivate > during omp lowering > Do you mean to say I should manually insert a firstprivate for iter and not the system figure out that it is shared? > 4) the printing looks weird for _Cilk_for, as I said earlier, the clauses > should > probably be printed after the closing ) of _Cilk_for rather than after nothing > on the previous line; also, there is no {} printed around the _Cilk_for body > and the next line is weirdly indented > Ok will look into this. > But more importantly, if I create some testcase with a generic C++ > conforming iterator (copied over from libgomp/testsuite/libgomp.c++/for- > 1.C), as in the second testcase, the *.gimple dump shows that _Cilk_for is > still > around the #pragma omp parallel. > The intent of the second testcase is that you can really eyeball all the > ctors/dtors/copy ctors etc. that should happen, and for -O0 shouldn't be > really inlined. > > Jakub