diff mbox

PR 60586

Message ID BF230D13CA30DD48930C31D4099330003B0F1015@fmsmsx101.amr.corp.intel.com
State New
Headers show

Commit Message

Iyer, Balaji V Sept. 2, 2015, 4:30 a.m. UTC
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

Comments

Jeff Law Sept. 2, 2015, 8:55 p.m. UTC | #1
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
diff mbox

Patch

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;
+}