diff mbox

Properly check for _Cilk_spawn in return stmt (PR c/60197)

Message ID 20140217203351.GH8907@redhat.com
State New
Headers show

Commit Message

Marek Polacek Feb. 17, 2014, 8:33 p.m. UTC 
On Mon, Feb 17, 2014 at 05:51:08PM +0000, Iyer, Balaji V wrote:
> > Regtested/bootstrapped on x86_64-linux, ok for 5.0?
> 
> 5.0? you mean 4.9 right?... since this is a minor bug-fix.
 
No, I meant 5.0, since this isn't a regression.  But maybe it could go
even into 4.9.  RM's call.
  
> I would move these two functions to cilk.c file instead of array-notations-common.c since cilk.c contains all Cilk keyword handling routines.
 
Ok, moved.

> > diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index
> > f074ab1..6f2c6b7 100644
> > --- gcc/c-family/c-common.h
> > +++ gcc/c-family/c-common.h
> > @@ -1375,6 +1375,7 @@ extern void cilkplus_extract_an_triplets (vec<tree,
> > va_gc> *, size_t, size_t,
> >  					  vec<vec<an_parts> > *);
> >  extern vec <tree, va_gc> *fix_sec_implicit_args
> >    (location_t, vec <tree, va_gc> *, vec<an_loop_parts>, size_t, tree);
> > +extern bool contains_cilk_spawn_stmt (tree);
> > 
> 
> ...and put the prototype under /* In Cilk.c.  */ part).
 
This as well.

> Other than the two above comments, the patch looks OK to me. But I don't have approval rights. I assume it doesn't break any existing regressions in the cilk-plus suite (doesn't look like it though)?

Yeah, it passed regtesting.  Note that we also ICE on e.g.
int
foo (void)
{
  int i;
  i = (_Cilk_spawn foo ()) + 1;
  return i;
}

I don't know whether this is valid use of _Cilk_spawn though.  In any case, this
patch addresses only _Cilk_spawn in return statements.

2014-02-17  Marek Polacek  <polacek@redhat.com>

	PR c/60197
c-family/
	* cilk.c (contains_cilk_spawn_stmt): New function.
	(contains_cilk_spawn_stmt_walker): Likewise.
	* c-common.h (contains_cilk_spawn_stmt): Add declaration.
c/
	* c-typeck.c (c_finish_return): Call contains_cilk_spawn_stmt instead
	of checking tree code.
cp/
	* typeck.c (check_return_expr): Call contains_cilk_spawn_stmt instead
	of checking tree code.
testsuite/
	* c-c++-common/cilk-plus/CK/pr60197.c: New test.


	Marek

Comments

Iyer, Balaji V Feb. 18, 2014, 10:06 p.m. UTC | #1 
> Yeah, it passed regtesting.  Note that we also ICE on e.g.
> int
> foo (void)
> {
>   int i;
>   i = (_Cilk_spawn foo ()) + 1;
>   return i;
> }
> 
> I don't know whether this is valid use of _Cilk_spawn though.  In any case,
> this patch addresses only _Cilk_spawn in return statements.

This is invalid.

Thanks,

Balaji V. Iyer.
diff mbox

Patch

diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index f074ab1..1099b10 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1389,4 +1389,5 @@  extern tree make_cilk_frame (tree);
 extern tree create_cilk_function_exit (tree, bool, bool);
 extern tree cilk_install_body_pedigree_operations (tree);
 extern void cilk_outline (tree, tree *, void *);
+extern bool contains_cilk_spawn_stmt (tree);
 #endif /* ! GCC_C_COMMON_H */
diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c
index f2179dfc..13cebf8 100644
--- gcc/c-family/cilk.c
+++ gcc/c-family/cilk.c
@@ -1292,3 +1292,25 @@  build_cilk_sync (void)
   TREE_SIDE_EFFECTS (sync) = 1;
   return sync;
 }
+
+/* Helper for contains_cilk_spawn_stmt, callback for walk_tree.  Return
+   non-null tree if TP contains CILK_SPAWN_STMT.  */
+
+static tree
+contains_cilk_spawn_stmt_walker (tree *tp, int *, void *)
+{
+  if (TREE_CODE (*tp) == CILK_SPAWN_STMT)
+    return *tp;
+  else
+    return NULL_TREE;
+}
+
+/* Returns true if EXPR or any of its subtrees contain CILK_SPAWN_STMT
+   node.  */
+
+bool
+contains_cilk_spawn_stmt (tree expr)
+{
+  return walk_tree (&expr, contains_cilk_spawn_stmt_walker, NULL, NULL)
+	 != NULL_TREE;
+}
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index da6a6fc..23cdb0e 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9134,7 +9134,7 @@  c_finish_return (location_t loc, tree retval, tree origtype)
 	  return error_mark_node;
 	}
     }
-  if (flag_cilkplus && retval && TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus && retval && contains_cilk_spawn_stmt (retval))
     {
       error_at (loc, "use of %<_Cilk_spawn%> in a return statement is not "
 		"allowed");
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 5fc0e6b..566411f 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -8328,7 +8328,7 @@  check_return_expr (tree retval, bool *no_warning)
 
   *no_warning = false;
 
-  if (flag_cilkplus && retval && TREE_CODE (retval) == CILK_SPAWN_STMT)
+  if (flag_cilkplus && retval && contains_cilk_spawn_stmt (retval))
     {
       error_at (EXPR_LOCATION (retval), "use of %<_Cilk_spawn%> in a return "
 		"statement is not allowed");
diff --git gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
index e69de29..2b47d1e 100644
--- gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
+++ gcc/testsuite/c-c++-common/cilk-plus/CK/pr60197.c
@@ -0,0 +1,66 @@ 
+/* PR c/60197 */
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+extern int foo (void);
+extern int bar (int);
+
+int
+fn1 (void)
+{
+  return (_Cilk_spawn foo ()) * 2; /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn2 (void)
+{
+  return (_Cilk_spawn foo ()) > 2; /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn3 (int i, int j, int k)
+{
+  return ((((((_Cilk_spawn foo () + i) - j) * k) / j) | i) ^ k) ; /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn4 (int i, int j, int k)
+{
+  return (((((i - _Cilk_spawn foo ()) * k) / j) | i) ^ k); /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn5 (void)
+{
+  return _Cilk_spawn foo (); /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn6 (void)
+{
+  return _Cilk_spawn foo () + _Cilk_spawn foo (); /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn7 (void)
+{
+  return 5 % _Cilk_spawn foo (); /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn8 (void)
+{
+  return !_Cilk_spawn foo (); /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn9 (void)
+{
+  return foo () && _Cilk_spawn foo (); /* { dg-error "in a return statement is not allowed" } */
+}
+
+int
+fn10 (void)
+{
+  return bar (_Cilk_spawn foo ()); /* { dg-error "in a return statement is not allowed" } */
+}