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

["Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>]

Hi Jakub,
	Did you get a chance to look at this?

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: Iyer, Balaji V
> Sent: Monday, February 24, 2014 6:17 PM
> To: 'Jakub Jelinek'
> Cc: 'Jason Merrill'; 'Jeff Law'; 'Aldy Hernandez'; 'gcc-patches@gcc.gnu.org';
> 'rth@redhat.com'
> Subject: RE: [PING] [PATCH] _Cilk_for for C and C++
> 
> Hi Jakub,
> 	Please see my responses below.
> 
> > -----Original Message-----
> > From: Iyer, Balaji V
> > Sent: Thursday, February 20, 2014 11:38 PM
> > To: Jakub Jelinek
> > Cc: 'Jason Merrill'; 'Jeff Law'; 'Aldy Hernandez';
> > 'gcc-patches@gcc.gnu.org'; 'rth@redhat.com'
> > Subject: RE: [PING] [PATCH] _Cilk_for for C and C++
> >
> > Hi Jakub,
> > 	I have attached the fixed patch and have answered your questions
> > below.
> >
> > > -----Original Message-----
> > > From: Jakub Jelinek [mailto:jakub@redhat.com]
> > > Sent: Wednesday, February 19, 2014 6:24 AM
> > > To: Iyer, Balaji V
> > > Cc: 'Jason Merrill'; 'Jeff Law'; 'Aldy Hernandez';
> > > 'gcc-patches@gcc.gnu.org'; 'rth@redhat.com'
> > > Subject: Re: [PING] [PATCH] _Cilk_for for C and C++
> > >
> > > On Wed, Feb 19, 2014 at 04:43:06AM +0000, Iyer, Balaji V wrote:
> > > > Attached, please find a patch with the test case attached (for1.cc).
> > > > The patch is the same but the cp-changelog has been modified to
> > > > reflect the new test-case.  Is this OK to install?
> > >
> > > 1) have you tested the patch at all?  I see
> > > FAIL: g++.dg/gomp/for-1.C -std=c++98  (test for errors, line 27)
> > > FAIL: g++.dg/gomp/for-1.C -std=c++98 (test for excess errors)
> > > FAIL: g++.dg/gomp/for-1.C -std=c++11  (test for errors, line 27)
> > > FAIL: g++.dg/gomp/for-1.C -std=c++11 (test for excess errors)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++98 (internal compiler error)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++98  (test for warnings, line
> > > 30)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++98  (test for warnings, line
> > > 37)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++98  (test for warnings, line
> > > 40)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++98 (test for excess errors)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++11 (internal compiler error)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++11  (test for warnings, line
> > > 30)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++11  (test for warnings, line
> > > 37)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++11  (test for warnings, line
> > > 40)
> > > FAIL: g++.dg/gomp/for-19.C -std=gnu++11 (test for excess errors)
> > > regressions caused by the patch, that is of course unacceptable.
> > >
> >
> > Fixed. I apologize for them. I have confirmed that it is OK now.
> >
> >
> > > 2) try this updated cf3.cc, e.g. with -O2 -fcilkplus if you can't
> > > find out why calling something multiple times is a bad idea,
> > > actually the latest patch is even worse than the older one, you now
> > > create 3 calls to the end method and 3 calls to operator-.  There
> > > should be just one call to that, before the #pragma omp parallel
> > > obviously, anything that
> > doesn't do that is just bad.
> > > I don't see a point in having if clause on the _Cilk_for, just keep
> > > it on the #pragma omp parallel only, at ompexp time you can easily
> > > find it there, there is no point to check it again in the parallel
> > > body of the
> > function.
> > >
> >
> > I have removed the if-clause from the _Cilk_for and now it is just in
> > #pragma omp parallel.
> >
> > I have removed the 3rd operator-, but I am not able to remove the 2nd.
> > I am looking into it, but I am not able to do it. The thing is, first
> > operator- was for the if-clause, which I need to calculate the
> > loop-count for the
> > __cilkrts_cilk_for_64 function. The second one is not necessary
> > because the end-value does not matter for _Cilk_for since they will be
> > replaced with the low and high values.  I tried several things such as
> > stopping gimplifcation of the cond value, or replacing it with a
> > constant, etc and those are causing other problems elsewhere.
> >
> > 	 The thing is, I am not able to find a way to automate this. I can't
> > assume the cond's  end-value is same as count, because this is only
> > true if we have an iterator and the handle_omp_for_iterator function
> > modifies the cond value correctly. I need to use the count value
> > (retval.0) as retval.1 but count-value is computed at a later time
> > than handle_omp_for_iterator (since it does not have any knowledge
> > about the #pragma omp parallel). It is giving the correct answers for
> > the benchmarks and is removing the 2nd operator- when optimization is
> turned on for the inlinable operator-.
> >
> > Can you provide me some advice about how to do it?
> 
> I have fixed the issue. Now the 2nd operator- does not occur. Attached,
> please fixed a fixed patch. Is this OK for trunk?
> 
> Thanks,
> 
> Balaji V. Iyer.