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';
'r...@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'; 'r...@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:ja...@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'; 'r...@redhat.com'
Subject: Re: [PING] [PATCH] _Cilk_for for C and C++
On Wed, Feb 19, 2014 at 04:43:06AM +, 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.