From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76105 invoked by alias); 19 Jul 2016 18:56:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 76083 invoked by uid 89); 19 Jul 2016 18:56:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=caf, unlock, LOCK, UD:trans-intrinsic.c X-HELO: smtp.smtpout.orange.fr Received: from smtp05.smtpout.orange.fr (HELO smtp.smtpout.orange.fr) (80.12.242.127) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 19 Jul 2016 18:56:49 +0000 Received: from [192.168.1.10] ([90.105.161.57]) by mwinf5d52 with ME id Liwc1t0091EcHfe03iwi96; Tue, 19 Jul 2016 20:56:45 +0200 X-ME-Helo: [192.168.1.10] X-ME-Auth: bW9yaW4tbWlrYWVsQG9yYW5nZS5mcg== X-ME-Date: Tue, 19 Jul 2016 20:56:45 +0200 X-ME-IP: 90.105.161.57 Subject: Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508) To: Alessandro Fanfarillo , gfortran , gcc-patches , Paul Richard Thomas , Tobias Burnus References: From: Mikael Morin Message-ID: Date: Tue, 19 Jul 2016 18:56:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg01179.txt.bz2 Hello, this is mostly good in general, but is lacking tests. Especially, tests for successfull matching, and tests for every error you are adding in the patch (except maybe the -fcoarray= one). Also tests that the code executes successfullly with -fcoarray=single, and that it produces the right function calls with -fcoarray=lib. more specific comments below. Mikael Le 15/07/2016 à 19:34, Alessandro Fanfarillo a écrit : > Third *PING* > > 2016-07-04 16:46 GMT-06:00 Alessandro Fanfarillo : >> * PING * >> >> 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo : >>> * PING * >>> >>> 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo : >>>> Dear all, >>>> >>>> please find in attachment the first patch (of n) for the FAILED IMAGES >>>> capability defined in the coarray TS 18508. >>>> The patch adds support for three new intrinsic functions defined in >>>> the TS for simulating a failure (fail image), checking an image status >>>> (image_status) and getting the list of failed images (failed_images). >>>> The patch has been built and regtested on x86_64-pc-linux-gnu. >>>> >>>> Ok for trunk? >>>> >>>> Alessandro > > first_complete_patch.diff > > commit b3bca5b09f4cbcf18f2409dae2485a16a7c06498 > Author: Alessandro Fanfarillo > Date: Mon Jun 6 14:27:37 2016 -0600 > > First patch Failed Images CAF TS-18508 > > diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c > index f3a4a43..9f519ff 100644 > --- a/gcc/fortran/match.c > +++ b/gcc/fortran/match.c > @@ -1594,6 +1594,7 @@ gfc_match_if (gfc_statement *if_type) > match ("event post", gfc_match_event_post, ST_EVENT_POST) > match ("event wait", gfc_match_event_wait, ST_EVENT_WAIT) > match ("exit", gfc_match_exit, ST_EXIT) > + match ("fail image", gfc_match_fail_image, ST_FAIL_IMAGE) > match ("flush", gfc_match_flush, ST_FLUSH) > match ("forall", match_simple_forall, ST_FORALL) > match ("go to", gfc_match_goto, ST_GOTO) > @@ -3073,6 +3074,41 @@ gfc_match_event_wait (void) > return event_statement (ST_EVENT_WAIT); > } > > +/* Match a FAIl IMAGE statement */ > + > +static match > +fail_image_statement (gfc_statement st) > +{ > + if (flag_coarray == GFC_FCOARRAY_NONE) > + { > + gfc_fatal_error ("Coarrays disabled at %C, use %<-fcoarray=%> to enable"); > + return MATCH_ERROR; > + } > + > + if (gfc_match_char ('(') == MATCH_YES) > + goto syntax; > + > + if(st == ST_FAIL_IMAGE) > + new_st.op = EXEC_FAIL_IMAGE; > + else > + gcc_unreachable(); You can use gcc_assert (st == ST_FAIL_IMAGE); foo...; instead of if (st == ST_FAIL_IMAGE) foo...; else gcc_unreachable (); > + > + return MATCH_YES; > + > + syntax: > + gfc_syntax_error (st); > + > + return MATCH_ERROR; > +} > + > +match > +gfc_match_fail_image (void) > +{ > + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement at %C")) */ > + /* return MATCH_ERROR; */ > + Can this be uncommented? > + return fail_image_statement (ST_FAIL_IMAGE); > +} > > /* Match LOCK/UNLOCK statement. Syntax: > LOCK ( lock-variable [ , lock-stat-list ] ) > diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c > index 1aaf4e2..b2f5596 100644 > --- a/gcc/fortran/trans-intrinsic.c > +++ b/gcc/fortran/trans-intrinsic.c > @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr *expr) > m, lbound)); > } > > +static void > +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr) > +{ > + unsigned int num_args; > + tree *args,tmp; > + > + num_args = gfc_intrinsic_argument_list_length (expr); > + args = XALLOCAVEC (tree, num_args); > + > + gfc_conv_intrinsic_function_args (se, expr, args, num_args); > + > + if (flag_coarray == GFC_FCOARRAY_LIB) > + { Can everything be put under the if? Does it work with -fcoarray=single? > + tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_image_status, 2, > + args[0], build_int_cst (integer_type_node, -1)); > + se->expr = tmp; > + } > +} > > static void > diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c > index 7d3cf8c..ce0eae7 100644 > --- a/gcc/fortran/trans-stmt.c > +++ b/gcc/fortran/trans-stmt.c > @@ -674,6 +674,31 @@ gfc_trans_stop (gfc_code *code, bool error_stop) > return gfc_finish_block (&se.pre); > } > > +/* Translate the FAIL IMAGE statement. We have to translate this statement > + to a runtime library call. */ > + > +tree > +gfc_trans_fail_image (gfc_code *code ATTRIBUTE_UNUSED) > +{ > + tree gfc_int4_type_node = gfc_get_int_type (4); > + gfc_se se; > + tree tmp; > + > + /* Start a new block for this statement. */ > + gfc_init_se (&se, NULL); > + gfc_start_block (&se.pre); > + > + tmp = build_int_cst (gfc_int4_type_node, 0); This tmp doesn't seem to be used. > + tmp = build_call_expr_loc (input_location, > + gfor_fndecl_caf_fail_image, 1, > + build_int_cst (pchar_type_node, 0)); > + > + gfc_add_expr_to_block (&se.pre, tmp); > + > + gfc_add_block_to_block (&se.pre, &se.post); > + > + return gfc_finish_block (&se.pre); > +} > > tree > gfc_trans_lock_unlock (gfc_code *code, gfc_exec_op op)