* [PATCH] PR 60586
@ 2015-09-01 0:05 Iyer, Balaji V
2015-09-01 22:26 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Iyer, Balaji V @ 2015-09-01 0:05 UTC (permalink / raw)
To: gcc-patches; +Cc: Zamyatin, Igor
[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]
Hello Everyone,
This patch will fix the bug reported in Bugzilla, PR 60586. The issue was that the spawned function's function arguments must not be pushed into the nested/lambda function. This patch should fix that issue.
I have tested this on x86_64 (linux and Cygwin flavors). Is this OK for trunk?
Here are the Changelog entries:
gcc/c-family/ChangeLog
2015-08-31 Balaji V. Iyer <balaji.v.iyer@intel.com>
PR middle-end/60586
* c-common.h: Added two more parameters to the gimplify_cilk_spawn
function.
* c-gimplify.c (c_gimplify_expr): Likewise.
* cilk.c (gimplify_cilk_spawn): Likewise and called
gimplify_call_params_in_spawned_fn.
(gimplify_call_params_in_spawned_fn): New function.
gcc/cp/ChangeLog
2015-08-31 Balaji V. Iyer <balaji.v.iyer@intel.com>
PR middle-end/60586
* cp-gimplify.c (cp_gimplify_expr): Added two additional parameters to
gimplify_cilk_spawn.
gcc/testsuite/ChangeLog
2015-08-31 Balaji V. Iyer <balaji.v.iyer@intel.com>
PR middle-end/60586
* c-c++-common/cilk-plus/CK/pr60586.c: New file.
* g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
Thanks,
Balaji V. Iyer.
[-- Attachment #2: diff.txt --]
[-- Type: text/plain, Size: 8131 bytes --]
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be63cd2..027bea6 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1423,7 +1423,7 @@ extern vec <tree, va_gc> *fix_sec_implicit_args
/* In cilk.c. */
extern tree insert_cilk_frame (tree);
extern void cilk_init_builtins (void);
-extern int gimplify_cilk_spawn (tree *);
+extern int gimplify_cilk_spawn (tree *, gimple_seq *, gimple_seq *);
extern void cilk_install_body_with_frame_cleanup (tree, tree, void *);
extern bool cilk_detect_spawn_and_unwrap (tree *);
extern bool cilk_set_spawn_marker (location_t, tree);
diff --git a/gcc/c-family/c-gimplify.c b/gcc/c-family/c-gimplify.c
index 1c81773..fd954d3 100644
--- a/gcc/c-family/c-gimplify.c
+++ b/gcc/c-family/c-gimplify.c
@@ -288,7 +288,8 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
/* If errors are seen, then just process it as a CALL_EXPR. */
if (!seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
case MODIFY_EXPR:
case INIT_EXPR:
@@ -299,7 +300,8 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
original expression (MODIFY/INIT/CALL_EXPR) is processes as
it is supposed to be. */
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
default:;
}
diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c
index 1012a4f..1fe6685 100644
--- a/gcc/c-family/cilk.c
+++ b/gcc/c-family/cilk.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "tree-iterator.h"
#include "tree-inline.h"
+#include "cp/cp-tree.h"
#include "c-family/c-common.h"
#include "toplev.h"
#include "tm.h"
@@ -762,12 +763,37 @@ create_cilk_wrapper (tree exp, tree *args_out)
return fndecl;
}
+/* Gimplify all the parameters for the Spawned function. *EXPR_P can be a
+ CALL_EXPR, INIT_EXPR, MODIFY_EXPR or TARGET_EXPR. *PRE_P and *POST_P are
+ gimple sequences from the caller of gimplify_cilk_spawn. */
+
+static void
+gimplify_call_params_in_spawned_fn (tree *expr_p, gimple_seq *pre_p,
+ gimple_seq *post_p)
+{
+ int ii = 0;
+ tree *fix_parm_expr = expr_p;
+ if ((TREE_CODE (*expr_p) == INIT_EXPR)
+ || (TREE_CODE (*expr_p) == TARGET_EXPR)
+ || (TREE_CODE (*expr_p) == MODIFY_EXPR))
+ fix_parm_expr = &TREE_OPERAND (*expr_p, 1);
+
+ if (TREE_CODE (*fix_parm_expr) == CALL_EXPR)
+ for (ii = 0; ii < call_expr_nargs (*fix_parm_expr); ii++)
+ gimplify_expr (&CALL_EXPR_ARG (*fix_parm_expr, ii), pre_p, post_p,
+ is_gimple_reg, fb_rvalue);
+ else if (TREE_CODE (*fix_parm_expr) == AGGR_INIT_EXPR)
+ for (ii = 0; ii < aggr_init_expr_nargs (*fix_parm_expr); ii++)
+ gimplify_expr (&AGGR_INIT_EXPR_ARG (*fix_parm_expr, ii), pre_p, post_p,
+ is_gimple_reg, fb_rvalue);
+}
+
/* Transform *SPAWN_P, a spawned CALL_EXPR, to gimple. *SPAWN_P can be a
CALL_EXPR, INIT_EXPR or MODIFY_EXPR. Returns GS_OK if everything is fine,
and GS_UNHANDLED, otherwise. */
int
-gimplify_cilk_spawn (tree *spawn_p)
+gimplify_cilk_spawn (tree *spawn_p, gimple_seq *pre_p, gimple_seq *post_p)
{
tree expr = *spawn_p;
tree function, call1, call2, new_args;
@@ -784,6 +810,8 @@ gimplify_cilk_spawn (tree *spawn_p)
while (TREE_CODE (expr) == CLEANUP_POINT_EXPR
|| TREE_CODE (expr) == EXPR_STMT)
expr = TREE_OPERAND (expr, 0);
+
+ gimplify_call_params_in_spawned_fn (&expr, pre_p, post_p);
new_args = NULL;
function = create_cilk_wrapper (expr, &new_args);
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index c36d339..8aacbbb 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -603,7 +603,8 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
if (fn_contains_cilk_spawn_p (cfun)
&& cilk_detect_spawn_and_unwrap (expr_p)
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
cp_gimplify_init_expr (expr_p);
if (TREE_CODE (*expr_p) != INIT_EXPR)
return GS_OK;
@@ -614,7 +615,8 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
if (fn_contains_cilk_spawn_p (cfun)
&& cilk_detect_spawn_and_unwrap (expr_p)
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
/* If the back end isn't clever enough to know that the lhs and rhs
types are the same, add an explicit conversion. */
@@ -715,13 +717,15 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
/* If errors are seen, then just process it as a CALL_EXPR. */
if (!seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
case CALL_EXPR:
if (fn_contains_cilk_spawn_p (cfun)
&& cilk_detect_spawn_and_unwrap (expr_p)
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p, pre_p,
+ post_p);
/* DR 1030 says that we need to evaluate the elements of an
initializer-list in forward order even when it's used as arguments to
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60586.c b/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60586.c
new file mode 100644
index 0000000..c4012a0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60586.c
@@ -0,0 +1,28 @@
+/* { dg-do run { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "-fcilkplus -O2" } */
+/* { dg-additional-options "-lcilkrts" { target { i?86-*-* x86_64-*-* } } } */
+
+int noop(int x)
+{
+ return x;
+}
+
+int post_increment(int *x)
+{
+ return (*x)++;
+}
+
+int main(int argc, char *argv[])
+{
+ int m = 5;
+ int n = m;
+ int r = _Cilk_spawn noop(post_increment(&n));
+ int n2 = n;
+ _Cilk_sync;
+
+ if (r != m || n2 != m + 1)
+ return 1;
+ else
+ return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/cilk-plus/CK/pr60586.cc b/gcc/testsuite/g++.dg/cilk-plus/CK/pr60586.cc
new file mode 100644
index 0000000..6a27cad
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cilk-plus/CK/pr60586.cc
@@ -0,0 +1,89 @@
+/* { dg-options "-fcilkplus" } */
+/* { dg-do run { target i?86-*-* x86_64-*-* arm*-*-* } } */
+/* { dg-options "-fcilkplus -lcilkrts" { target { i?86-*-* x86_64-*-* } } } */
+
+class Rectangle
+{
+ int area_val, h, w;
+ public:
+ Rectangle (int, int);
+ Rectangle (int, int, int);
+ ~Rectangle ();
+ int area ();
+};
+Rectangle::~Rectangle ()
+{
+ h = 0;
+ w = 0;
+ area_val = 0;
+}
+Rectangle::Rectangle (int height, int width)
+{
+ h = height;
+ w = width;
+ area_val = 0;
+}
+
+int some_func(int &x)
+{
+ x++;
+ return x;
+}
+
+Rectangle::Rectangle (int height, int width, int area_orig)
+{
+ h = height;
+ w = width;
+ area_val = area_orig;
+}
+
+int Rectangle::area()
+{
+ return (area_val += (h*w));
+}
+
+
+int some_func (int &);
+
+/* Spawning constructor. */
+int main1 (void)
+{
+ int x = 3;
+ Rectangle r = _Cilk_spawn Rectangle (some_func(x), 3);
+ return r.area();
+}
+
+/* Spawning constructor 2. */
+int main2 (void)
+{
+ Rectangle r (_Cilk_spawn Rectangle (4, 3));
+ return r.area();
+}
+
+/* Spawning copy constructor. */
+int main3 (void)
+{
+ int x = 3;
+ Rectangle r = _Cilk_spawn Rectangle (some_func(x), 3, 2);
+ return r.area ();
+}
+
+/* Spawning copy constructor 2. */
+int main4 (void)
+{
+ Rectangle r ( _Cilk_spawn Rectangle (4, 3, 2));
+ return r.area();
+}
+
+int main (void)
+{
+ if (main1 () != 12)
+ __builtin_abort ();
+ if (main2 () != 12)
+ __builtin_abort ();
+ if (main3 () != 14)
+ __builtin_abort ();
+ if (main4() != 14)
+ __builtin_abort ();
+ return 0;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR 60586
2015-09-01 0:05 [PATCH] PR 60586 Iyer, Balaji V
@ 2015-09-01 22:26 ` Jeff Law
2015-09-02 1:16 ` Iyer, Balaji V
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jeff Law @ 2015-09-01 22:26 UTC (permalink / raw)
To: Iyer, Balaji V, gcc-patches; +Cc: Zamyatin, Igor
On 08/31/2015 06:04 PM, Iyer, Balaji V wrote:
> Hello Everyone,
> This patch will fix the bug reported in Bugzilla, PR 60586. The issue was that the spawned function's function arguments must not be pushed into the nested/lambda function. This patch should fix that issue.
>
> I have tested this on x86_64 (linux and Cygwin flavors). Is this OK for trunk?
>
> Here are the Changelog entries:
>
> gcc/c-family/ChangeLog
> 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
>
> PR middle-end/60586
> * c-common.h: Added two more parameters to the gimplify_cilk_spawn
> function.
> * c-gimplify.c (c_gimplify_expr): Likewise.
> * cilk.c (gimplify_cilk_spawn): Likewise and called
> gimplify_call_params_in_spawned_fn.
> (gimplify_call_params_in_spawned_fn): New function.
>
> gcc/cp/ChangeLog
> 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
>
> PR middle-end/60586
> * cp-gimplify.c (cp_gimplify_expr): Added two additional parameters to
> gimplify_cilk_spawn.
>
> gcc/testsuite/ChangeLog
> 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
>
> PR middle-end/60586
> * c-c++-common/cilk-plus/CK/pr60586.c: New file.
> * g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
>
>
> Thanks,
>
> Balaji V. Iyer.
>
>
> diff.txt
>
>
> diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c
> index 1012a4f..1fe6685 100644
> --- a/gcc/c-family/cilk.c
> +++ b/gcc/c-family/cilk.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> #include "gimplify.h"
> #include "tree-iterator.h"
> #include "tree-inline.h"
> +#include "cp/cp-tree.h"
Presumably you needed this for AGGR_INIT_EXPR. I believe you need to
refactor the code a bit -- including a C++ front-end header file into
something in c-family seems wrong.
While it may not break in this specific instance, but it is a modularity
violation.
> #include "c-family/c-common.h"
> #include "toplev.h"
> #include "tm.h"
> @@ -762,12 +763,37 @@ create_cilk_wrapper (tree exp, tree *args_out)
> return fndecl;
> }
>
> +/* Gimplify all the parameters for the Spawned function. *EXPR_P can be a
> + CALL_EXPR, INIT_EXPR, MODIFY_EXPR or TARGET_EXPR. *PRE_P and *POST_P are
> + gimple sequences from the caller of gimplify_cilk_spawn. */
Comment doesn't match the code, code also checks for AGGR_INIT_EXPR.
Given the dependency on AGGR_INIT_EXPR, it seems this checking somehow
needs to move into the front-ends.
I think once that's fixed this ought to be ready for the trunk.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PR 60586
2015-09-01 22:26 ` Jeff Law
@ 2015-09-02 1:16 ` Iyer, Balaji V
2015-09-02 1:22 ` Iyer, Balaji V
2015-09-02 4:30 ` Iyer, Balaji V
2 siblings, 0 replies; 6+ messages in thread
From: Iyer, Balaji V @ 2015-09-02 1:16 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: Zamyatin, Igor
> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Tuesday, September 1, 2015 3:26 PM
> To: Iyer, Balaji V; gcc-patches@gcc.gnu.org
> Cc: Zamyatin, Igor
> Subject: Re: [PATCH] PR 60586
>
> On 08/31/2015 06:04 PM, Iyer, Balaji V wrote:
> > Hello Everyone,
> > This patch will fix the bug reported in Bugzilla, PR 60586. The issue
> was that the spawned function's function arguments must not be pushed
> into the nested/lambda function. This patch should fix that issue.
> >
> > I have tested this on x86_64 (linux and Cygwin flavors). Is this OK for trunk?
> >
> > Here are the Changelog entries:
> >
> > gcc/c-family/ChangeLog
> > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> >
> > PR middle-end/60586
> > * c-common.h: Added two more parameters to the
> gimplify_cilk_spawn
> > function.
> > * c-gimplify.c (c_gimplify_expr): Likewise.
> > * cilk.c (gimplify_cilk_spawn): Likewise and called
> > gimplify_call_params_in_spawned_fn.
> > (gimplify_call_params_in_spawned_fn): New function.
> >
> > gcc/cp/ChangeLog
> > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> >
> > PR middle-end/60586
> > * cp-gimplify.c (cp_gimplify_expr): Added two additional parameters
> to
> > gimplify_cilk_spawn.
> >
> > gcc/testsuite/ChangeLog
> > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> >
> > PR middle-end/60586
> > * c-c++-common/cilk-plus/CK/pr60586.c: New file.
> > * g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
> >
> >
> > Thanks,
> >
> > Balaji V. Iyer.
> >
> >
> > diff.txt
> >
> >
> > diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c index
> > 1012a4f..1fe6685 100644
> > --- a/gcc/c-family/cilk.c
> > +++ b/gcc/c-family/cilk.c
> > @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "gimplify.h"
> > #include "tree-iterator.h"
> > #include "tree-inline.h"
> > +#include "cp/cp-tree.h"
> Presumably you needed this for AGGR_INIT_EXPR. I believe you need to
> refactor the code a bit -- including a C++ front-end header file into something
> in c-family seems wrong.
>
> While it may not break in this specific instance, but it is a modularity violation.
>
>
>
> > #include "c-family/c-common.h"
> > #include "toplev.h"
> > #include "tm.h"
> > @@ -762,12 +763,37 @@ create_cilk_wrapper (tree exp, tree *args_out)
> > return fndecl;
> > }
> >
> > +/* Gimplify all the parameters for the Spawned function. *EXPR_P can be
> a
> > + CALL_EXPR, INIT_EXPR, MODIFY_EXPR or TARGET_EXPR. *PRE_P and
> *POST_P are
> > + gimple sequences from the caller of gimplify_cilk_spawn. */
> Comment doesn't match the code, code also checks for AGGR_INIT_EXPR.
>
> Given the dependency on AGGR_INIT_EXPR, it seems this checking
> somehow
> needs to move into the front-ends.
Will you be OK if I make the function "gimplify_call_params_in_spawned_fn" part of the targetm structure?
Thanks,
Balaji V. Iyer.
>
> I think once that's fixed this ought to be ready for the trunk.
>
> jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PR 60586
2015-09-01 22:26 ` Jeff Law
2015-09-02 1:16 ` Iyer, Balaji V
@ 2015-09-02 1:22 ` Iyer, Balaji V
2015-09-02 4:30 ` Iyer, Balaji V
2 siblings, 0 replies; 6+ messages in thread
From: Iyer, Balaji V @ 2015-09-02 1:22 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: Zamyatin, Igor
> -----Original Message-----
> From: Iyer, Balaji V
> Sent: Tuesday, September 1, 2015 6:17 PM
> To: 'Jeff Law'; gcc-patches@gcc.gnu.org
> Cc: Zamyatin, Igor
> Subject: RE: [PATCH] PR 60586
>
>
>
> > -----Original Message-----
> > From: Jeff Law [mailto:law@redhat.com]
> > Sent: Tuesday, September 1, 2015 3:26 PM
> > To: Iyer, Balaji V; gcc-patches@gcc.gnu.org
> > Cc: Zamyatin, Igor
> > Subject: Re: [PATCH] PR 60586
> >
> > On 08/31/2015 06:04 PM, Iyer, Balaji V wrote:
> > > Hello Everyone,
> > > This patch will fix the bug reported in Bugzilla, PR 60586. The
> > > issue
> > was that the spawned function's function arguments must not be pushed
> > into the nested/lambda function. This patch should fix that issue.
> > >
> > > I have tested this on x86_64 (linux and Cygwin flavors). Is this OK for
> trunk?
> > >
> > > Here are the Changelog entries:
> > >
> > > gcc/c-family/ChangeLog
> > > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> > >
> > > PR middle-end/60586
> > > * c-common.h: Added two more parameters to the
> > gimplify_cilk_spawn
> > > function.
> > > * c-gimplify.c (c_gimplify_expr): Likewise.
> > > * cilk.c (gimplify_cilk_spawn): Likewise and called
> > > gimplify_call_params_in_spawned_fn.
> > > (gimplify_call_params_in_spawned_fn): New function.
> > >
> > > gcc/cp/ChangeLog
> > > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> > >
> > > PR middle-end/60586
> > > * cp-gimplify.c (cp_gimplify_expr): Added two additional
> > > parameters
> > to
> > > gimplify_cilk_spawn.
> > >
> > > gcc/testsuite/ChangeLog
> > > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> > >
> > > PR middle-end/60586
> > > * c-c++-common/cilk-plus/CK/pr60586.c: New file.
> > > * g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
> > >
> > >
> > > Thanks,
> > >
> > > Balaji V. Iyer.
> > >
> > >
> > > diff.txt
> > >
> > >
> > > diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c index
> > > 1012a4f..1fe6685 100644
> > > --- a/gcc/c-family/cilk.c
> > > +++ b/gcc/c-family/cilk.c
> > > @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> > > #include "gimplify.h"
> > > #include "tree-iterator.h"
> > > #include "tree-inline.h"
> > > +#include "cp/cp-tree.h"
> > Presumably you needed this for AGGR_INIT_EXPR. I believe you need to
> > refactor the code a bit -- including a C++ front-end header file into
> > something in c-family seems wrong.
> >
> > While it may not break in this specific instance, but it is a modularity
> violation.
> >
> >
> >
> > > #include "c-family/c-common.h"
> > > #include "toplev.h"
> > > #include "tm.h"
> > > @@ -762,12 +763,37 @@ create_cilk_wrapper (tree exp, tree *args_out)
> > > return fndecl;
> > > }
> > >
> > > +/* Gimplify all the parameters for the Spawned function. *EXPR_P
> > > +can be
> > a
> > > + CALL_EXPR, INIT_EXPR, MODIFY_EXPR or TARGET_EXPR. *PRE_P and
> > *POST_P are
> > > + gimple sequences from the caller of gimplify_cilk_spawn. */
> > Comment doesn't match the code, code also checks for AGGR_INIT_EXPR.
> >
> > Given the dependency on AGGR_INIT_EXPR, it seems this checking
> somehow
> > needs to move into the front-ends.
>
> Will you be OK if I make the function "gimplify_call_params_in_spawned_fn"
> part of the targetm structure?
>
Sorry, I meant lang_hooks.
> Thanks,
>
> Balaji V. Iyer.
>
> >
> > I think once that's fixed this ought to be ready for the trunk.
> >
> > jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PR 60586
2015-09-01 22:26 ` Jeff Law
2015-09-02 1:16 ` Iyer, Balaji V
2015-09-02 1:22 ` Iyer, Balaji V
@ 2015-09-02 4:30 ` Iyer, Balaji V
2015-09-02 20:58 ` Jeff Law
2 siblings, 1 reply; 6+ messages in thread
From: Iyer, Balaji V @ 2015-09-02 4:30 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: Zamyatin, Igor
[-- Attachment #1: Type: text/plain, Size: 4030 bytes --]
Hi Jeff,
I thought about this for a minute and I don't think I need to use the lang_hooks. I could do this change right before calling gimplify_cilk_spawn. I have attached the fixed patch and have answered your questions below. Here are the ChangeLog entries:
gcc/c-family/ChangeLog:
2015-09-01 Balaji V. Iyer <balaji.v.iyer@intel.com>
PR middle-end/60586
* c-common.h (cilk_gimplify_call_params_in_spawned_fn): New prototype.
* c-gimplify.c (c_gimplify_expr): Added a call to the function
cilk_gimplify_call_params_in_spawned_fn.
* cilk.c (cilk_gimplify_call_params_in_spawned_fn): New function.
(gimplify_cilk_spawn): Removed EXPR_STMT and CLEANUP_POINT_EXPR
unwrapping.
gcc/cp/ChangeLog
2015-09-01 Balaji V. Iyer <balaji.v.iyer@intel.com>
PR middle-end/60586
* cp-gimplify.c (cilk_cp_gimplify_call_params_in_spawned_fn): New
function.
(cp_gimplify_expr): Added a call to the function
cilk_cp_gimplify_call_params_in_spawned_fn.
gcc/testsuite/ChangeLog
2015-09-01 Balaji V. Iyer <balaji.v.iyer@intel.com>
PR middle-end/60586
* c-c++-common/cilk-plus/CK/pr60586.c: New file.
* g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
Is this OK for trunk?
Thanks,
Balaji V. Iyer.
> > Here are the Changelog entries:
> >
> > gcc/c-family/ChangeLog
> > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> >
> > PR middle-end/60586
> > * c-common.h: Added two more parameters to the
> gimplify_cilk_spawn
> > function.
> > * c-gimplify.c (c_gimplify_expr): Likewise.
> > * cilk.c (gimplify_cilk_spawn): Likewise and called
> > gimplify_call_params_in_spawned_fn.
> > (gimplify_call_params_in_spawned_fn): New function.
> >
> > gcc/cp/ChangeLog
> > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> >
> > PR middle-end/60586
> > * cp-gimplify.c (cp_gimplify_expr): Added two additional parameters
> to
> > gimplify_cilk_spawn.
> >
> > gcc/testsuite/ChangeLog
> > 2015-08-31 Balaji V. Iyer<balaji.v.iyer@intel.com>
> >
> > PR middle-end/60586
> > * c-c++-common/cilk-plus/CK/pr60586.c: New file.
> > * g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
> >
> >
> > Thanks,
> >
> > Balaji V. Iyer.
> >
> >
> > diff.txt
> >
> >
> > diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c index
> > 1012a4f..1fe6685 100644
> > --- a/gcc/c-family/cilk.c
> > +++ b/gcc/c-family/cilk.c
> > @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "gimplify.h"
> > #include "tree-iterator.h"
> > #include "tree-inline.h"
> > +#include "cp/cp-tree.h"
> Presumably you needed this for AGGR_INIT_EXPR. I believe you need to
> refactor the code a bit -- including a C++ front-end header file into something
> in c-family seems wrong.
>
I removed it.
> While it may not break in this specific instance, but it is a modularity violation.
>
Fixed!
>
>
> > #include "c-family/c-common.h"
> > #include "toplev.h"
> > #include "tm.h"
> > @@ -762,12 +763,37 @@ create_cilk_wrapper (tree exp, tree *args_out)
> > return fndecl;
> > }
> >
> > +/* Gimplify all the parameters for the Spawned function. *EXPR_P can be
> a
> > + CALL_EXPR, INIT_EXPR, MODIFY_EXPR or TARGET_EXPR. *PRE_P and
> *POST_P are
> > + gimple sequences from the caller of gimplify_cilk_spawn. */
> Comment doesn't match the code, code also checks for AGGR_INIT_EXPR.
>
Fixed (sort of not applicable as I restructured as you suggested above).
> Given the dependency on AGGR_INIT_EXPR, it seems this checking
> somehow
> needs to move into the front-ends.
>
Yep, moved them to c-gimplify.c and cp-gimplify.c (the caller functions of gimplify_cilk_spawn).
> I think once that's fixed this ought to be ready for the trunk.
>
So, is this OK for trunk?
> jeff
[-- Attachment #2: diff2.txt --]
[-- Type: text/plain, Size: 9175 bytes --]
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be63cd2..d24dfda 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1424,6 +1424,8 @@ extern vec <tree, va_gc> *fix_sec_implicit_args
extern tree insert_cilk_frame (tree);
extern void cilk_init_builtins (void);
extern int gimplify_cilk_spawn (tree *);
+extern void cilk_gimplify_call_params_in_spawned_fn (tree *, gimple_seq *,
+ gimple_seq *);
extern void cilk_install_body_with_frame_cleanup (tree, tree, void *);
extern bool cilk_detect_spawn_and_unwrap (tree *);
extern bool cilk_set_spawn_marker (location_t, tree);
diff --git a/gcc/c-family/c-gimplify.c b/gcc/c-family/c-gimplify.c
index 1c81773..92987b5 100644
--- a/gcc/c-family/c-gimplify.c
+++ b/gcc/c-family/c-gimplify.c
@@ -288,8 +288,10 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
/* If errors are seen, then just process it as a CALL_EXPR. */
if (!seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-
+ {
+ cilk_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ }
case MODIFY_EXPR:
case INIT_EXPR:
case CALL_EXPR:
@@ -299,7 +301,10 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
original expression (MODIFY/INIT/CALL_EXPR) is processes as
it is supposed to be. */
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ {
+ cilk_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ }
default:;
}
diff --git a/gcc/c-family/cilk.c b/gcc/c-family/cilk.c
index 1012a4f..1c316a4 100644
--- a/gcc/c-family/cilk.c
+++ b/gcc/c-family/cilk.c
@@ -762,6 +762,34 @@ create_cilk_wrapper (tree exp, tree *args_out)
return fndecl;
}
+/* Gimplify all the parameters for the Spawned function. *EXPR_P can be a
+ CALL_EXPR, INIT_EXPR, MODIFY_EXPR or TARGET_EXPR. *PRE_P and *POST_P are
+ gimple sequences from the caller of gimplify_cilk_spawn. */
+
+void
+cilk_gimplify_call_params_in_spawned_fn (tree *expr_p, gimple_seq *pre_p,
+ gimple_seq *post_p)
+{
+ int ii = 0;
+ tree *fix_parm_expr = expr_p;
+
+ /* Remove CLEANUP_POINT_EXPR and EXPR_STMT from *spawn_p. */
+ while (TREE_CODE (*fix_parm_expr) == CLEANUP_POINT_EXPR
+ || TREE_CODE (*fix_parm_expr) == EXPR_STMT)
+ *fix_parm_expr = TREE_OPERAND (*fix_parm_expr, 0);
+
+ if ((TREE_CODE (*expr_p) == INIT_EXPR)
+ || (TREE_CODE (*expr_p) == TARGET_EXPR)
+ || (TREE_CODE (*expr_p) == MODIFY_EXPR))
+ fix_parm_expr = &TREE_OPERAND (*expr_p, 1);
+
+ if (TREE_CODE (*fix_parm_expr) == CALL_EXPR)
+ for (ii = 0; ii < call_expr_nargs (*fix_parm_expr); ii++)
+ gimplify_expr (&CALL_EXPR_ARG (*fix_parm_expr, ii), pre_p, post_p,
+ is_gimple_reg, fb_rvalue);
+}
+
+
/* Transform *SPAWN_P, a spawned CALL_EXPR, to gimple. *SPAWN_P can be a
CALL_EXPR, INIT_EXPR or MODIFY_EXPR. Returns GS_OK if everything is fine,
and GS_UNHANDLED, otherwise. */
@@ -779,12 +807,6 @@ gimplify_cilk_spawn (tree *spawn_p)
cfun->calls_cilk_spawn = 1;
cfun->is_cilk_function = 1;
-
- /* Remove CLEANUP_POINT_EXPR and EXPR_STMT from *spawn_p. */
- while (TREE_CODE (expr) == CLEANUP_POINT_EXPR
- || TREE_CODE (expr) == EXPR_STMT)
- expr = TREE_OPERAND (expr, 0);
-
new_args = NULL;
function = create_cilk_wrapper (expr, &new_args);
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index c36d339..5ab0604 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -95,6 +95,25 @@ finish_bc_block (tree *block, enum bc_t bc, tree label)
DECL_CHAIN (label) = NULL_TREE;
}
+/* This function is a wrapper for cilk_gimplify_call_params_in_spawned_fn.
+ *EXPR_P can be a CALL_EXPR, INIT_EXPR, MODIFY_EXPR, AGGR_INIT_EXPR or
+ TARGET_EXPR. *PRE_P and *POST_P are gimple sequences from the caller
+ of gimplify_cilk_spawn. */
+
+static void
+cilk_cp_gimplify_call_params_in_spawned_fn (tree *expr_p, gimple_seq *pre_p,
+ gimple_seq *post_p)
+{
+ int ii = 0;
+
+ cilk_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ if (TREE_CODE (*expr_p) == AGGR_INIT_EXPR)
+ for (ii = 0; ii < aggr_init_expr_nargs (*expr_p); ii++)
+ gimplify_expr (&AGGR_INIT_EXPR_ARG (*expr_p, ii), pre_p, post_p,
+ is_gimple_reg, fb_rvalue);
+}
+
+
/* Get the LABEL_EXPR to represent a break or continue statement
in the current block scope. BC indicates which. */
@@ -603,7 +622,10 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
if (fn_contains_cilk_spawn_p (cfun)
&& cilk_detect_spawn_and_unwrap (expr_p)
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ {
+ cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ }
cp_gimplify_init_expr (expr_p);
if (TREE_CODE (*expr_p) != INIT_EXPR)
return GS_OK;
@@ -614,8 +636,10 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
if (fn_contains_cilk_spawn_p (cfun)
&& cilk_detect_spawn_and_unwrap (expr_p)
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-
+ {
+ cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ }
/* If the back end isn't clever enough to know that the lhs and rhs
types are the same, add an explicit conversion. */
tree op0 = TREE_OPERAND (*expr_p, 0);
@@ -715,14 +739,18 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
/* If errors are seen, then just process it as a CALL_EXPR. */
if (!seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-
+ {
+ cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ }
case CALL_EXPR:
if (fn_contains_cilk_spawn_p (cfun)
&& cilk_detect_spawn_and_unwrap (expr_p)
&& !seen_error ())
- return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-
+ {
+ cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
+ return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ }
/* DR 1030 says that we need to evaluate the elements of an
initializer-list in forward order even when it's used as arguments to
a constructor. So if the target wants to evaluate them in reverse
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60586.c b/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60586.c
new file mode 100644
index 0000000..c4012a0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60586.c
@@ -0,0 +1,28 @@
+/* { dg-do run { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "-fcilkplus -O2" } */
+/* { dg-additional-options "-lcilkrts" { target { i?86-*-* x86_64-*-* } } } */
+
+int noop(int x)
+{
+ return x;
+}
+
+int post_increment(int *x)
+{
+ return (*x)++;
+}
+
+int main(int argc, char *argv[])
+{
+ int m = 5;
+ int n = m;
+ int r = _Cilk_spawn noop(post_increment(&n));
+ int n2 = n;
+ _Cilk_sync;
+
+ if (r != m || n2 != m + 1)
+ return 1;
+ else
+ return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/cilk-plus/CK/pr60586.cc b/gcc/testsuite/g++.dg/cilk-plus/CK/pr60586.cc
new file mode 100644
index 0000000..6a27cad
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cilk-plus/CK/pr60586.cc
@@ -0,0 +1,89 @@
+/* { dg-options "-fcilkplus" } */
+/* { dg-do run { target i?86-*-* x86_64-*-* arm*-*-* } } */
+/* { dg-options "-fcilkplus -lcilkrts" { target { i?86-*-* x86_64-*-* arm*-*-* } } } */
+
+class Rectangle
+{
+ int area_val, h, w;
+ public:
+ Rectangle (int, int);
+ Rectangle (int, int, int);
+ ~Rectangle ();
+ int area ();
+};
+Rectangle::~Rectangle ()
+{
+ h = 0;
+ w = 0;
+ area_val = 0;
+}
+Rectangle::Rectangle (int height, int width)
+{
+ h = height;
+ w = width;
+ area_val = 0;
+}
+
+int some_func(int &x)
+{
+ x++;
+ return x;
+}
+
+Rectangle::Rectangle (int height, int width, int area_orig)
+{
+ h = height;
+ w = width;
+ area_val = area_orig;
+}
+
+int Rectangle::area()
+{
+ return (area_val += (h*w));
+}
+
+
+int some_func (int &);
+
+/* Spawning constructor. */
+int main1 (void)
+{
+ int x = 3;
+ Rectangle r = _Cilk_spawn Rectangle (some_func(x), 3);
+ return r.area();
+}
+
+/* Spawning constructor 2. */
+int main2 (void)
+{
+ Rectangle r (_Cilk_spawn Rectangle (4, 3));
+ return r.area();
+}
+
+/* Spawning copy constructor. */
+int main3 (void)
+{
+ int x = 3;
+ Rectangle r = _Cilk_spawn Rectangle (some_func(x), 3, 2);
+ return r.area ();
+}
+
+/* Spawning copy constructor 2. */
+int main4 (void)
+{
+ Rectangle r ( _Cilk_spawn Rectangle (4, 3, 2));
+ return r.area();
+}
+
+int main (void)
+{
+ if (main1 () != 12)
+ __builtin_abort ();
+ if (main2 () != 12)
+ __builtin_abort ();
+ if (main3 () != 14)
+ __builtin_abort ();
+ if (main4() != 14)
+ __builtin_abort ();
+ return 0;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR 60586
2015-09-02 4:30 ` Iyer, Balaji V
@ 2015-09-02 20:58 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2015-09-02 20:58 UTC (permalink / raw)
To: Iyer, Balaji V, gcc-patches; +Cc: Zamyatin, Igor
On 09/01/2015 10:30 PM, Iyer, Balaji V wrote:
> Hi Jeff,
> I thought about this for a minute and I don't think I need to use the lang_hooks. I could do this change right before calling gimplify_cilk_spawn. I have attached the fixed patch and have answered your questions below. Here are the ChangeLog entries:
>
> gcc/c-family/ChangeLog:
> 2015-09-01 Balaji V. Iyer <balaji.v.iyer@intel.com>
>
> PR middle-end/60586
> * c-common.h (cilk_gimplify_call_params_in_spawned_fn): New prototype.
> * c-gimplify.c (c_gimplify_expr): Added a call to the function
> cilk_gimplify_call_params_in_spawned_fn.
> * cilk.c (cilk_gimplify_call_params_in_spawned_fn): New function.
> (gimplify_cilk_spawn): Removed EXPR_STMT and CLEANUP_POINT_EXPR
> unwrapping.
>
> gcc/cp/ChangeLog
> 2015-09-01 Balaji V. Iyer <balaji.v.iyer@intel.com>
>
> PR middle-end/60586
> * cp-gimplify.c (cilk_cp_gimplify_call_params_in_spawned_fn): New
> function.
> (cp_gimplify_expr): Added a call to the function
> cilk_cp_gimplify_call_params_in_spawned_fn.
>
> gcc/testsuite/ChangeLog
> 2015-09-01 Balaji V. Iyer <balaji.v.iyer@intel.com>
>
> PR middle-end/60586
> * c-c++-common/cilk-plus/CK/pr60586.c: New file.
> * g++.dg/cilk-plus/CK/pr60586.cc: Likewise.
>
> Is this OK for trunk?
Yes. Please install.
Thanks,
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-02 20:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-01 0:05 [PATCH] PR 60586 Iyer, Balaji V
2015-09-01 22:26 ` Jeff Law
2015-09-02 1:16 ` Iyer, Balaji V
2015-09-02 1:22 ` Iyer, Balaji V
2015-09-02 4:30 ` Iyer, Balaji V
2015-09-02 20:58 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).