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

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

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?

Thanks,

Balaji V. Iyer.

Attachment: diff.txt
Description: diff.txt

Attachment: c-ChangeLogs
Description: c-ChangeLogs

Attachment: cp-ChangeLogs
Description: cp-ChangeLogs