This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][PR c++/80038][6/7 Regression] Destroy temps for _Cilk_spawn calling in the child


Hi,

After r227423, GCC begun to destroy temps for cilkplus spawned functions in the parent, instead
of in the child.  This violated Intel Cilk specs
<https://www.cilkplus.org/sites/default/files/open_specifications/Intel_Cilk_plus_lang_spec_1.2.htm>
and broke some valid code such as NumericMonoid
<https://github.com/hivert/NumericMonoid/issues/2>.

To fix this, revert r227423 and re-fix PR60586 without moving out the destruction of temps by
inserting __cilkrts_detach call in the exact location of GIMPLE, instead of tree (where we can
not separate argument evaluation and copying with function call). 

The pedigree operations are merged into built-in __cilkrts_detach (just like libcilkrts cilk_fake.h).

Bootstrapped and regtested on x86_64-linux-gnu.

The patch has 500+ lines so I attach it.  ChangeLog is pasted here:

2017-03-24  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	PR c++/80038
	* c-family/c-common.h (cilk_gimplify_call_params_in_spawned_fn,
	  cilk_install_body_pedigree_operations): Remove prototypes.
	* c-family/c-gimplify.c (c_gimplify_expr): Remove the calls to
	  the function cilk_gimplify_call_params_in_spawned_fn.
	* c-family/cilk.c: (cilk_set_spawn_marker): Mark the function
	  calls which should be detached.
	  (cilk_gimplify_call_params_in_spawned_fn,
	   cilk_install_body_pedigree_operations): Remove function.
	  (gimplify_cilk_spawn): Add EXPR_STMT and CLEANUP_POINT_EXPR
	  unwrapping.
	* c/c-typeck.c (cilk_install_body_with_frame_cleanup):
	  Don't add pedigree operation and detach call here.
	* cp/cp-cilkplus.c (cilk_install_body_with_frame_cleanup): Ditto.
	* cp/cp-gimplify.c (cilk_cp_gimplify_call_params_in_spawned_fn):
	  Remove function.
	  (cp_gimplify_expr): Remove the calls to the function
	  cilk_cp_gimplify_call_params_in_spawned_fn.
	* cp/semantics.c: Preserve the flag of function calls should
	  be detached.
	* cilk_common.c (expand_builtin_cilk_detach): Move pedigree
	  operations here.
	* gimplify.c (gimplify_cilk_detach): New static function.
	  (gimplify_call_expr, gimplify_modify_expr): Call it for the
	  function calls should be detached.
	* lto/lto-lang.c (lto_init): Set in_lto_p earlier.
	* tree-core.h: Document new macro EXPR_CILK_SPAWN.
	* tree.h: Add new macro EXPR_CILK_SPAWN.
	* testsuite/g++.dg/cilk-plus/CK/pr80038.cc: New test.
-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University
diff --git a/gcc/c-family/c-gimplify.c b/gcc/c-family/c-gimplify.c
index 57edb41..1ae75d2 100644
--- a/gcc/c-family/c-gimplify.c
+++ b/gcc/c-family/c-gimplify.c
@@ -280,10 +280,7 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
 		 && cilk_detect_spawn_and_unwrap (expr_p));
 
       if (!seen_error ())
-	{
-	  cilk_gimplify_call_params_in_spawned_fn (expr_p, pre_p);
-	  return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-	}
+        return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
       return GS_ERROR;
 
     case MODIFY_EXPR:
@@ -295,10 +292,7 @@ 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 ())
-	{
-	  cilk_gimplify_call_params_in_spawned_fn (expr_p, pre_p);
-	  return (enum gimplify_status) gimplify_cilk_spawn (expr_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 43478ff..b58fb99 100644
--- a/gcc/c-family/cilk.c
+++ b/gcc/c-family/cilk.c
@@ -109,6 +109,10 @@ cilk_set_spawn_marker (location_t loc, tree fcall)
   else
     {
       cfun->calls_cilk_spawn = true;
+      if (TREE_CODE (fcall) == CALL_EXPR)
+        EXPR_CILK_SPAWN (fcall) = 1;
+      else /* TREE_CODE (fcall) == TARGET_EXPR */
+        EXPR_CILK_SPAWN (TREE_OPERAND (fcall, 1)) = 1;
       return true;
     }
 }
@@ -775,37 +779,6 @@ 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)
-{
-  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)
-    {
-      /* Cilk outlining assumes GENERIC bodies, avoid leaking SSA names
-         via parameters.  */
-      for (ii = 0; ii < call_expr_nargs (*fix_parm_expr); ii++)
-	gimplify_arg (&CALL_EXPR_ARG (*fix_parm_expr, ii), pre_p,
-		      EXPR_LOCATION (*fix_parm_expr), false);
-    }
-}
-
-
 /* 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.  */
@@ -823,6 +796,12 @@ 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/c/c-typeck.c b/gcc/c/c-typeck.c
index b4f61b0..5d4d70b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -14203,14 +14203,8 @@ cilk_install_body_with_frame_cleanup (tree fndecl, tree body, void *w)
   add_local_decl (cfun, frame);
   
   DECL_SAVED_TREE (fndecl) = list;
-  tree frame_ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (frame)), 
-			   frame);
-  tree body_list = cilk_install_body_pedigree_operations (frame_ptr);
-  gcc_assert (TREE_CODE (body_list) == STATEMENT_LIST);
-  
-  tree detach_expr = build_call_expr (cilk_detach_fndecl, 1, frame_ptr); 
-  append_to_statement_list (detach_expr, &body_list);
 
+  tree body_list = alloc_stmt_list ();
   cilk_outline (fndecl, &body, (struct wrapper_data *) w);
   body = fold_build_cleanup_point_expr (void_type_node, body);
 
diff --git a/gcc/cilk-common.c b/gcc/cilk-common.c
index 46626b7..9cbe03f 100644
--- a/gcc/cilk-common.c
+++ b/gcc/cilk-common.c
@@ -365,11 +365,60 @@ expand_builtin_cilk_detach (tree exp)
   tree worker = cilk_dot (fptr, CILK_TI_FRAME_WORKER, 0);
   tree tail = cilk_arrow (worker, CILK_TI_WORKER_TAIL, 1);
 
+  tree faddr = build1 (ADDR_EXPR, cilk_frame_ptr_type_decl, fptr);
+  tree enter_frame = build_call_expr (cilk_enter_fast_fndecl, 1, faddr);
+  expand_expr (enter_frame, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
+  tree pedigree = cilk_dot (fptr, CILK_TI_FRAME_PEDIGREE, 0);
+  tree pedigree_rank = cilk_dot (pedigree, CILK_TI_PEDIGREE_RANK, 0);
+  tree parent_pedigree = cilk_dot (pedigree, CILK_TI_PEDIGREE_PARENT, 0);
+  tree pedigree_parent = cilk_arrow (parent, CILK_TI_FRAME_PEDIGREE, 0);
+  tree pedigree_parent_rank = cilk_dot (pedigree_parent,
+					CILK_TI_PEDIGREE_RANK, 0);
+  tree pedigree_parent_parent = cilk_dot (pedigree_parent,
+				     CILK_TI_PEDIGREE_PARENT, 0);
+  tree worker_pedigree = cilk_arrow (worker, CILK_TI_WORKER_PEDIGREE, 1);
+  tree w_pedigree_rank = cilk_dot (worker_pedigree, CILK_TI_PEDIGREE_RANK, 0);
+  tree w_pedigree_parent = cilk_dot (worker_pedigree,
+				     CILK_TI_PEDIGREE_PARENT, 0);
+
   rtx wreg = expand_expr (worker, NULL_RTX, Pmode, EXPAND_NORMAL);
   if (GET_CODE (wreg) != REG)
     wreg = copy_to_reg (wreg);
   rtx preg = expand_expr (parent, NULL_RTX, Pmode, EXPAND_NORMAL);
 
+  /* sf.pedigree.rank = worker->pedigree.rank.  */
+  tree exp1 = build2 (MODIFY_EXPR, void_type_node, pedigree_rank,
+		     w_pedigree_rank);
+  expand_expr (exp1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
+  /* sf.pedigree.parent = worker->pedigree.parent.  */
+  exp1 = build2 (MODIFY_EXPR, void_type_node, parent_pedigree,
+		 w_pedigree_parent);
+  expand_expr (exp1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
+  /* sf.call_parent->pedigree.rank = worker->pedigree.rank.  */
+  exp1 = build2 (MODIFY_EXPR, void_type_node, pedigree_parent_rank,
+		 w_pedigree_rank);
+  expand_expr (exp1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
+  /* sf.call_parent->pedigree.parent = worker->pedigree.parent.  */
+  exp1 = build2 (MODIFY_EXPR, void_type_node, pedigree_parent_parent,
+		 w_pedigree_parent);
+  expand_expr (exp1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
+  /* sf->worker.pedigree.rank = 0.  */
+  exp1 = build2 (MODIFY_EXPR, void_type_node, w_pedigree_rank,
+		 build_zero_cst (uint64_type_node));
+  expand_expr (exp1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
+  /* sf->pedigree.parent = &sf->pedigree.  */
+  exp1 = build2 (MODIFY_EXPR, void_type_node, w_pedigree_parent,
+		 build1 (ADDR_EXPR,
+			 build_pointer_type (cilk_pedigree_type_decl),
+			 pedigree));
+  expand_expr (exp1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+
   /* TMP <- WORKER.TAIL
     *TMP <- PARENT
      TMP <- TMP + 1
diff --git a/gcc/cp/cp-cilkplus.c b/gcc/cp/cp-cilkplus.c
index d147e7e..7c66448 100644
--- a/gcc/cp/cp-cilkplus.c
+++ b/gcc/cp/cp-cilkplus.c
@@ -223,11 +223,7 @@ cilk_install_body_with_frame_cleanup (tree fndecl, tree orig_body, void *wd)
   location_t loc = EXPR_LOCATION (orig_body);
   tree list = alloc_stmt_list ();
   DECL_SAVED_TREE (fndecl) = list;
-  tree fptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (frame)), frame);
-  tree body = cilk_install_body_pedigree_operations (fptr);
-  gcc_assert (TREE_CODE (body) == STATEMENT_LIST);
-  tree detach_expr = build_call_expr (cilk_detach_fndecl, 1, fptr);
-  append_to_statement_list (detach_expr, &body);
+  tree body = alloc_stmt_list ();
   cilk_outline (fndecl, &orig_body, (struct wrapper_data *) wd);
   append_to_statement_list (orig_body, &body);
   if (flag_exceptions)
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index ebb5da9..91d67c0 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -88,25 +88,6 @@ 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);
-  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.  */
 
@@ -648,11 +629,7 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
       if (fn_contains_cilk_spawn_p (cfun))
 	{
 	  if (cilk_cp_detect_spawn_and_unwrap (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);
-	    }
+	    return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
 	  if (seen_error () && contains_cilk_spawn_stmt (*expr_p))
 	    return GS_ERROR;
 	}
@@ -667,10 +644,7 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	if (fn_contains_cilk_spawn_p (cfun)
 	    && cilk_cp_detect_spawn_and_unwrap (expr_p)
 	    && !seen_error ())
-	  {
-	    cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
-	    return (enum gimplify_status) gimplify_cilk_spawn (expr_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);
@@ -788,20 +762,14 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 		 && cilk_cp_detect_spawn_and_unwrap (expr_p));
 
       if (!seen_error ())
-	{
-	  cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
-	  return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-	}
+        return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
       return GS_ERROR;
 
     case CALL_EXPR:
       if (fn_contains_cilk_spawn_p (cfun)
 	  && cilk_cp_detect_spawn_and_unwrap (expr_p)
 	  && !seen_error ())
-	{
-	  cilk_cp_gimplify_call_params_in_spawned_fn (expr_p, pre_p, post_p);
-	  return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
-	}
+        return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
       ret = GS_OK;
       if (!CALL_EXPR_FN (*expr_p))
 	/* Internal function call.  */;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index bcfdd66..7f67c9f 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -4102,6 +4102,8 @@ simplify_aggr_init_expr (tree *tp)
     = CALL_EXPR_OPERATOR_SYNTAX (aggr_init_expr);
   CALL_EXPR_ORDERED_ARGS (call_expr) = CALL_EXPR_ORDERED_ARGS (aggr_init_expr);
   CALL_EXPR_REVERSE_ARGS (call_expr) = CALL_EXPR_REVERSE_ARGS (aggr_init_expr);
+  /* Preserve CILK_SPAWN flag.  */
+  EXPR_CILK_SPAWN (call_expr) = EXPR_CILK_SPAWN (aggr_init_expr);
 
   if (style == ctor)
     {
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index fbf136f..547507a 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3069,6 +3069,19 @@ maybe_fold_stmt (gimple_stmt_iterator *gsi)
   return fold_stmt (gsi);
 }
 
+/* Add a gimple call to __builtin_cilk_detach to GIMPLE sequence PRE_P,
+   with the pointer to the proper cilk frame.  */
+static void
+gimplify_cilk_detach (gimple_seq *pre_p)
+{
+  tree frame = cfun->cilk_frame_decl;
+  tree ptrf = build1 (ADDR_EXPR, cilk_frame_ptr_type_decl,
+		      frame);
+  gcall *detach = gimple_build_call (cilk_detach_fndecl, 1,
+				     ptrf);
+  gimplify_seq_add_stmt(pre_p, detach);
+}
+
 /* Gimplify the CALL_EXPR node *EXPR_P into the GIMPLE sequence PRE_P.
    WANT_VALUE is true if the result of the call is desired.  */
 
@@ -3105,6 +3118,9 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
 			EXPR_LOCATION (*expr_p));
 	  vargs.quick_push (CALL_EXPR_ARG (*expr_p, i));
 	}
+
+      if (EXPR_CILK_SPAWN (*expr_p))
+        gimplify_cilk_detach (pre_p);
       gimple *call = gimple_build_call_internal_vec (ifn, vargs);
       gimplify_seq_add_stmt (pre_p, call);
       return GS_ALL_DONE;
@@ -3336,6 +3352,8 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
       call = gimple_build_call_from_tree (*expr_p);
       gimple_call_set_fntype (call, TREE_TYPE (fnptrtype));
       notice_special_calls (call);
+      if (EXPR_CILK_SPAWN (*expr_p))
+        gimplify_cilk_detach (pre_p);
       gimplify_seq_add_stmt (pre_p, call);
       gsi = gsi_last (*pre_p);
       maybe_fold_stmt (&gsi);
@@ -5610,6 +5628,9 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	   SSA name w/o a definition.  We may have uses in the GIMPLE IL.
 	   ???  This doesn't make it a default-def.  */
 	SSA_NAME_DEF_STMT (*to_p) = gimple_build_nop ();
+
+      if (EXPR_CILK_SPAWN (*from_p))
+        gimplify_cilk_detach (pre_p);
       assign = call_stmt;
     }
   else
diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c
index ca8945e..52ab2a8 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -1201,6 +1201,9 @@ lto_init (void)
 {
   int i;
 
+  /* Initialize LTO-specific data structures.  */
+  in_lto_p = true;
+
   /* We need to generate LTO if running in WPA mode.  */
   flag_generate_lto = (flag_wpa != NULL);
 
@@ -1283,9 +1286,6 @@ lto_init (void)
       }
 #undef NAME_TYPE
 
-  /* Initialize LTO-specific data structures.  */
-  in_lto_p = true;
-
   return true;
 }
 
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 2b1759e..efed993 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1194,6 +1194,10 @@ struct GTY(()) tree_base {
        SSA_NAME_OCCURS_IN_ABNORMAL_PHI in
            SSA_NAME
 
+       EXPR_CILK_SPAWN in
+           CALL_EXPR
+           AGGR_INIT_EXPR
+
    used_flag:
 
        TREE_USED in
diff --git a/gcc/tree.h b/gcc/tree.h
index aa137e4..d274015 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -894,6 +894,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
 /* Cilk keywords accessors.  */
 #define CILK_SPAWN_FN(NODE) TREE_OPERAND (CILK_SPAWN_STMT_CHECK (NODE), 0)
 
+/* If this is true, we should insert a __cilk_detach call just before
+   this function call.  */
+#define EXPR_CILK_SPAWN(NODE) \
+  (tree_check2 (NODE, __FILE__, __LINE__, __FUNCTION__, \
+                CALL_EXPR, AGGR_INIT_EXPR)->base.u.bits.unsigned_flag)
+
 /* In a RESULT_DECL, PARM_DECL and VAR_DECL, means that it is
    passed by invisible reference (and the TREE_TYPE is a pointer to the true
    type).  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]