This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PING] [PATCH] _Cilk_for for C and C++
- From: Jakub Jelinek <jakub at redhat dot com>
- To: "Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>
- Cc: "'Jason Merrill'" <jason at redhat dot com>, "'Jeff Law'" <law at redhat dot com>, "'Aldy Hernandez'" <aldyh at redhat dot com>, "'gcc-patches at gcc dot gnu dot org'" <gcc-patches at gcc dot gnu dot org>, "'rth at redhat dot com'" <rth at redhat dot com>
- Date: Wed, 12 Feb 2014 15:59:13 +0100
- Subject: Re: [PING] [PATCH] _Cilk_for for C and C++
- Authentication-results: sourceware.org; auth=none
- References: <BF230D13CA30DD48930C31D4099330003A4C8892 at FMSMSX101 dot amr dot corp dot intel dot com> <BF230D13CA30DD48930C31D4099330003A4C8E1C at FMSMSX101 dot amr dot corp dot intel dot com> <20140129113129 dot GT892 at tucnak dot redhat dot com> <BF230D13CA30DD48930C31D4099330003A4CC12A at FMSMSX101 dot amr dot corp dot intel dot com> <20140207140233 dot GA3184 at laptop dot redhat dot com> <BF230D13CA30DD48930C31D4099330003A4CCC35 at FMSMSX101 dot amr dot corp dot intel dot com> <20140207145301 dot GE2764 at laptop dot redhat dot com> <BF230D13CA30DD48930C31D4099330003A4CCDA4 at FMSMSX101 dot amr dot corp dot intel dot com> <20140210175731 dot GL20378 at tucnak dot redhat dot com> <BF230D13CA30DD48930C31D4099330003A4CD72B at FMSMSX101 dot amr dot corp dot intel dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Feb 10, 2014 at 10:07:18PM +0000, Iyer, Balaji V wrote:
> I looked at both but forgot to test them with my implementation. Sorry
> about this. I have fixed the ICE issue. To make sure this does not
> happen further, I have added your test cf3.C into test suite (renamed to
> cf3.cc). I hope that is OK with you.
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 have attached a fixed patch and Changelogs. Is this OK?
Looks better (note, still looking just at the dumps), but not completely ok
yet. On cf3.cc, I see in *.gimple:
D.2883 = J<int>::begin (j);
I<int>::I (&i, D.2883);
D.2885 = J<int>::end (j);
retval.0 = operator-<int> (D.2885, &i);
D.2886 = retval.0 / 2;
#pragma omp parallel firstprivate(i) if(D.2886) shared(D.2865) shared(j)
{
difference_type retval.1;
const struct I & D.2888;
const difference_type D.2866;
long int D.2889;
struct I & D.2890;
try
{
_Cilk_for (D.2864 = 0; D.2864 < retval.1; D.2864 = D.2864 + 2) private(D.2864)
{
D.2889 = D.2864 - D.2865;
D.2866 = D.2889;
try
{
D.2890 = I<int>::operator+= (&i, &D.2866);
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.
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.
Jakub