From: Mikael Morin <morin-mikael@orange.fr>
To: Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>,
gfortran <fortran@gcc.gnu.org>,
gcc-patches <gcc-patches@gcc.gnu.org>,
Paul Richard Thomas <paul.richard.thomas@gmail.com>,
Tobias Burnus <burnus@net-b.de>
Subject: Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Date: Tue, 19 Jul 2016 18:57:00 -0000 [thread overview]
Message-ID: <a7fe21ba-bd71-90e8-e1ef-624dea6bbf02@orange.fr> (raw)
In-Reply-To: <CAHqFgjWGccPnNKJnsiyV1Dv3uPrDLY-uxptgnJMCOEwYC5X-9g@mail.gmail.com>
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 <fanfarillo.gcc@gmail.com>:
>> * PING *
>>
>> 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>:
>>> * PING *
>>>
>>> 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>:
>>>> 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 <fanfarillo@ing.uniroma2.it>
> 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)
next prev parent reply other threads:[~2016-07-19 18:57 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-06 21:05 Alessandro Fanfarillo
2016-06-21 16:59 ` Alessandro Fanfarillo
2016-07-04 22:46 ` Alessandro Fanfarillo
2016-07-15 17:34 ` Alessandro Fanfarillo
2016-07-19 18:57 ` Mikael Morin [this message]
2016-07-20 9:39 ` Andre Vehreschild
2016-07-20 19:18 ` Mikael Morin
2016-07-21 19:05 ` Alessandro Fanfarillo
2016-08-04 3:09 ` Alessandro Fanfarillo
2016-08-09 11:23 ` Paul Richard Thomas
2016-08-09 17:44 ` Alessandro Fanfarillo
2016-09-07 21:01 ` Alessandro Fanfarillo
[not found] ` <CAHqFgjXbwQQnnZp5N+WtWnxNxWducGcU9QSdHRhCdPwNf1tdBQ@mail.gmail.com>
2016-09-19 15:55 ` Andre Vehreschild
2016-09-21 18:04 ` Alessandro Fanfarillo
2016-09-28 13:13 ` Alessandro Fanfarillo
2016-08-08 17:12 ` Dan Nagle
2017-01-18 6:42 Damian Rouson
[not found] <1474481042.70029.ezmlm@gcc.gnu.org>
[not found] ` <FD49FDC8-1AAF-4ED4-BB07-734F323AEA34@sourceryinstitute.org>
2017-01-18 17:20 ` Andre Vehreschild
2017-01-18 17:55 ` Alessandro Fanfarillo
[not found] ` <CAKT_9NXOrmL0m2pX-wgk7V2WnAJMd8eJvp+UYvMJHQs-QEMdOA@mail.gmail.com>
2017-01-18 18:01 ` Andre Vehreschild
2017-01-18 18:18 ` Alessandro Fanfarillo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a7fe21ba-bd71-90e8-e1ef-624dea6bbf02@orange.fr \
--to=morin-mikael@orange.fr \
--cc=burnus@net-b.de \
--cc=fanfarillo.gcc@gmail.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=paul.richard.thomas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).