From patchwork Mon Feb 17 20:33:51 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 321128 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id F31FF2C00D0 for ; Tue, 18 Feb 2014 07:34:08 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=XE/RMDqQYOnXUu1hH cOIfwTJHDXKijP/P5E1Pxit9J2AmubU3rLyTkWKvHP3jSiaGGkMblCIwgfgm27jy reiuNMFPE8ChLobJyKkNeQV/d+siK1mfZLNNRunI3V68Fk7ne8kW0MvrK450pktb vKvPA8AmJZ/cPm5DfLufIuPvEQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=uiupOHGtwh61Q/bJH7OhwS4 z2Y4=; b=trvK+PN1OSLWu0CeJtFQ/nnOVaTCgPOsgMzAq6BnSpWeCKdYCnNBmV6 isFV2bOp/zTpiQ+dSK3rKH3n8GI0s16M1JjR6K/HA94f1hwatTw4DECsqYk3TMre tehSLmdrXJma/sV7hMnxqsS0BKD1Ob8OxU+ZYyIhVH/9d/1eNpBs= Received: (qmail 29050 invoked by alias); 17 Feb 2014 20:34:00 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 29035 invoked by uid 89); 17 Feb 2014 20:33:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Feb 2014 20:33:59 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1HKXtt9018190 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 17 Feb 2014 15:33:56 -0500 Received: from redhat.com (ovpn-116-40.ams2.redhat.com [10.36.116.40]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s1HKXqWt019499 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 17 Feb 2014 15:33:54 -0500 Date: Mon, 17 Feb 2014 21:33:51 +0100 From: Marek Polacek To: "Iyer, Balaji V" Cc: "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] Properly check for _Cilk_spawn in return stmt (PR c/60197) Message-ID: <20140217203351.GH8907@redhat.com> References: <20140217174230.GG8907@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) 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 > va_gc> *, size_t, size_t, > > vec > *); > > extern vec *fix_sec_implicit_args > > (location_t, vec *, vec, 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 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 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" } */ +}