From patchwork Wed Sep 2 04:30:11 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Iyer, Balaji V" X-Patchwork-Id: 513290 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 E08E01401F0 for ; Wed, 2 Sep 2015 14:30:25 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=yylm26h1; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; q=dns; s=default; b=IujJN0efFm0qNzfo vl4QDzSBghPLTbL2vj+bQO1FuIe/B/ILWQWzdEBJHYicUlif8Fpj9r4iS48ZNKXo FNDds93BkRsuvrmlGlpzgyHsQLMSwuOaEPHRCtVTUMKLFv/fsZq4nSAi1M20aka6 meoOmLtBeaoOY/2Bg7Ntb/k5ji0= 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:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; s=default; bh=z52xEq2Gp3XIVdtrycHJfq sP9pM=; b=yylm26h1c5ArNDtvnq1morbI8ovhI8S0ieE2XMo8Vc2L5IPjake4Uf c0apCI6t/CfPsOk0saiinytPeqMOwQttQeEk22h3LZb/NxTJyAUGUlzLwbExkON1 u3cvQxSWCHpAliW1AUOYddOjy7h+MiAMIRRCy1mYvVeT3mp3n858U= Received: (qmail 128093 invoked by alias); 2 Sep 2015 04:30:17 -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 128082 invoked by uid 89); 2 Sep 2015 04:30:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.5 required=5.0 tests=AWL, BAYES_60, SPF_PASS, SUBJ_ALL_CAPS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mga09.intel.com Received: from mga09.intel.com (HELO mga09.intel.com) (134.134.136.24) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Sep 2015 04:30:14 +0000 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 01 Sep 2015 21:30:14 -0700 X-ExtLoop1: 1 Received: from orsmsx110.amr.corp.intel.com ([10.22.240.8]) by orsmga002.jf.intel.com with ESMTP; 01 Sep 2015 21:30:13 -0700 Received: from orsmsx161.amr.corp.intel.com (10.22.240.84) by ORSMSX110.amr.corp.intel.com (10.22.240.8) with Microsoft SMTP Server (TLS) id 14.3.224.2; Tue, 1 Sep 2015 21:30:12 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by ORSMSX161.amr.corp.intel.com (10.22.240.84) with Microsoft SMTP Server (TLS) id 14.3.224.2; Tue, 1 Sep 2015 21:30:12 -0700 Received: from fmsmsx101.amr.corp.intel.com ([169.254.1.27]) by fmsmsx116.amr.corp.intel.com ([169.254.2.108]) with mapi id 14.03.0224.002; Tue, 1 Sep 2015 21:30:12 -0700 From: "Iyer, Balaji V" To: Jeff Law , "gcc-patches@gcc.gnu.org" CC: "Zamyatin, Igor" Subject: RE: [PATCH] PR 60586 Date: Wed, 2 Sep 2015 04:30:11 +0000 Message-ID: References: <55E62613.7060706@redhat.com> In-Reply-To: <55E62613.7060706@redhat.com> MIME-Version: 1.0 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); + } 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; +}