Re: [PATCH] PR 60586
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 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 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 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
RE: [PATCH] PR 60586
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
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
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
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
> >
> > 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
> >
> > 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
> >
> > 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
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 *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);
RE: [PATCH] PR 60586
> -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:l...@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 > > > > > > 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 > > > > > > 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 > > > > > > 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
RE: [PATCH] PR 60586
> -Original Message- > From: Jeff Law [mailto:l...@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 > > > > 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 > > > > 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 > > > > 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
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 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 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 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
